Merge pull request #1164 from algairim/smart-159

Mask velues of sensitive keys of shell.env configuration in env strea…
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractWindowsYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractWindowsYamlTest.java
index 4d9f07c..5a93cb1 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractWindowsYamlTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractWindowsYamlTest.java
@@ -67,7 +67,7 @@
 
             String stdin = getStreamOrFail(subTask, BrooklynTaskTags.STREAM_STDIN);
             String stdout = getStreamOrFail(subTask, BrooklynTaskTags.STREAM_STDOUT);
-            String stderr = getStreamOrFail(subTask, BrooklynTaskTags.STREAM_STDERR);
+            String stderr = getStream(subTask, BrooklynTaskTags.STREAM_STDERR);
             String env = getStream(subTask, BrooklynTaskTags.STREAM_ENV);
             String msg = "stdin="+stdin+"; stdout="+stdout+"; stderr="+stderr+"; env="+env;
 
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WindowsYamlLiveTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WindowsYamlLiveTest.java
index 2be6149..a84822a 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WindowsYamlLiveTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WindowsYamlLiveTest.java
@@ -105,8 +105,8 @@
                 "location:",
                 "  byon:",
                 "    hosts:",
-                "    - winrm: "+machine.getAddress().getHostAddress() 
-                        // this is the default, probably not necessary but kept for posterity 
+                "    - winrm: "+machine.getAddress().getHostAddress()
+                        // this is the default, probably not necessary but kept for posterity
                         +":5985",
                 "      password: "+JavaStringEscapes.wrapJavaString(machine.config().get(WinRmMachineLocation.PASSWORD)),
                 "      user: "+machine.config().get(WinRmMachineLocation.USER),
@@ -141,12 +141,12 @@
             app = null;
         }
     }
-    
+
     @Override
     protected ManagementContextInternal mgmt() {
         return (ManagementContextInternal) super.mgmt();
     }
-    
+
     @Test(groups="Live")
     public void testPowershellMinimalist() throws Exception {
         Map<String, String> cmds = ImmutableMap.<String, String>builder()
@@ -154,12 +154,12 @@
                 .put("launch.powershell.command", JavaStringEscapes.wrapJavaString("& \"$Env:INSTALL_DIR\\exit0.ps1\""))
                 .put("checkRunning.powershell.command", JavaStringEscapes.wrapJavaString("& \"$Env:INSTALL_DIR\\exit0.bat\""))
                 .build();
-        
+
         Map<String, List<String>> stdouts = ImmutableMap.of();
-        
+
         runWindowsApp(cmds, stdouts, true, null);
     }
-    
+
     @Test(groups="Live")
     public void testPowershell() throws Exception {
         Map<String, String> cmds = ImmutableMap.<String, String>builder()
@@ -174,17 +174,17 @@
                 .put("checkRunning.powershell.command", "\"& c:\\\\exit0.ps1\"")
                 .put("stop.powershell.command", "\"& c:\\\\exit0.ps1\"")
                 .build();
-        
+
         Map<String, List<String>> stdouts = ImmutableMap.<String, List<String>>builder()
                 .put("winrm: install.*", ImmutableList.of("myInstall"))
                 .put("winrm: post-install-command.*", ImmutableList.of("myPostInstall"))
                 .put("winrm: customize.*", ImmutableList.of("myval"))
                 .put("winrm: pre-launch-command.*", ImmutableList.of("myval"))
                 .build();
-        
+
         runWindowsApp(cmds, stdouts, false, null);
     }
-    
+
     @Test(groups="Live")
     public void testBatch() throws Exception {
         Map<String, String> cmds = ImmutableMap.<String, String>builder()
@@ -206,10 +206,10 @@
                 .put("winrm: customize.*", ImmutableList.of("myval"))
                 .put("winrm: pre-launch-command.*", ImmutableList.of("myval"))
                 .build();
-        
+
         runWindowsApp(cmds, stdouts, false, null);
     }
-    
+
     @Test(groups="Live")
     public void testPowershellExit1() throws Exception {
         Map<String, String> cmds = ImmutableMap.<String, String>builder()
@@ -224,12 +224,12 @@
                 .put("checkRunning.powershell.command", "\"& c:\\\\exit0.ps1\"")
                 .put("stop.powershell.command", "\"& c:\\\\exit0.ps1\"")
                 .build();
-        
+
         Map<String, List<String>> stdouts = ImmutableMap.of();
-        
+
         runWindowsApp(cmds, stdouts, false, "winrm: pre-install-command.*");
     }
-    
+
     // FIXME Failing to match the expected exception, but looks fine! Needs more investigation.
     @Test(groups="Live")
     public void testPowershellCheckRunningExit1() throws Exception {
@@ -245,12 +245,12 @@
                 .put("checkRunning.powershell.command", "\"& c:\\\\exit1.ps1\"")
                 .put("stop.powershell.command", "\"& c:\\\\exit0.ps1\"")
                 .build();
-        
+
         Map<String, List<String>> stdouts = ImmutableMap.of();
-        
+
         runWindowsApp(cmds, stdouts, false, "winrm: is-running-command.*");
     }
-    
+
     // FIXME Needs more work to get the stop's task that failed, so can assert got the right error message
     @Test(groups="Live")
     public void testPowershellStopExit1() throws Exception {
@@ -266,15 +266,15 @@
                 .put("checkRunning.powershell.command", "\"& c:\\\\exit0.ps1\"")
                 .put("stop.powershell.command", "\"& c:\\\\exit1.ps1\"")
                 .build();
-        
+
         Map<String, List<String>> stdouts = ImmutableMap.of();
-        
+
         runWindowsApp(cmds, stdouts, false, "winrm: stop-command.*");
     }
-    
+
     protected void runWindowsApp(Map<String, String> commands, Map<String, List<String>> stdouts, boolean useInstallDir, String taskRegexFailed) throws Exception {
         String cmdFailed = (taskRegexFailed == null) ? null : TASK_REGEX_TO_COMMAND.get(taskRegexFailed);
-        
+
         List<String> yaml = Lists.newArrayList();
         yaml.addAll(yamlLocation);
         String prefix = useInstallDir ? "" : "c:\\";
@@ -293,7 +293,7 @@
                 "      classpath://org/apache/brooklyn/camp/brooklyn/exit0.ps1: "+prefix+"exit0.ps1",
                 "      classpath://org/apache/brooklyn/camp/brooklyn/exit1.ps1: "+prefix+"exit1.ps1",
                 ""));
-        
+
         for (Map.Entry<String, String> entry : commands.entrySet()) {
             yaml.add("    "+entry.getKey()+": "+entry.getValue());
         }
@@ -303,12 +303,12 @@
             waitForApplicationTasks(app);
             log.info("App started:");
             Dumper.dumpInfo(app);
-            
+
             VanillaWindowsProcess entity = (VanillaWindowsProcess) app.getChildren().iterator().next();
-            
+
             EntityAsserts.assertAttributeEqualsEventually(entity, Attributes.SERVICE_UP, true);
             assertStreams(entity, stdouts);
-            
+
         } else if (cmdFailed.equals("stop-command")) {
             app = createAndStartApplication(Joiner.on("\n").join(yaml));
             waitForApplicationTasks(app);
@@ -316,10 +316,10 @@
             Dumper.dumpInfo(app);
             VanillaWindowsProcess entity = (VanillaWindowsProcess) app.getChildren().iterator().next();
             EntityAsserts.assertAttributeEqualsEventually(entity, Attributes.SERVICE_UP, true);
-            
+
             entity.stop();
             assertSubTaskFailures(entity, ImmutableMap.of(taskRegexFailed, StringPredicates.containsLiteral("for "+cmdFailed)));
-            
+
         } else {
             try {
                 app = createAndStartApplication(Joiner.on("\n").join(yaml));
@@ -337,23 +337,23 @@
         String in = "%KEY1%: %ADDR_RESOLVED%";
         yaml.addAll(ImmutableList.of(
             "services:",
-            "  - type: org.apache.brooklyn.entity.software.base.VanillaWindowsProcess", 
-            "    brooklyn.config:", 
-            "      install.command: "+JavaStringEscapes.wrapJavaString("echo "+in), 
-            "      customize.command: "+JavaStringEscapes.wrapJavaString("echo "+in), 
-            "      launch.command: "+JavaStringEscapes.wrapJavaString("echo "+in), 
-            "      stop.command: echo true", 
-            "      checkRunning.command: echo true", 
-            "      shell.env:", 
-            "        KEY1: Address", 
+            "  - type: org.apache.brooklyn.entity.software.base.VanillaWindowsProcess",
+            "    brooklyn.config:",
+            "      install.command: "+JavaStringEscapes.wrapJavaString("echo "+in),
+            "      customize.command: "+JavaStringEscapes.wrapJavaString("echo "+in),
+            "      launch.command: "+JavaStringEscapes.wrapJavaString("echo "+in),
+            "      stop.command: echo true",
+            "      checkRunning.command: echo true",
+            "      shell.env:",
+            "        KEY1: Address",
             "        ADDR_RESOLVED: $brooklyn:attributeWhenReady(\"host.address\")"));
 
         app = createAndStartApplication(Joiner.on("\n").join(yaml));
         waitForApplicationTasks(app);
         log.info("App started:");
         Dumper.dumpInfo(app);
-        
-        
+
+
         Entity win = Iterables.getOnlyElement(app.getChildren());
         String out = "Address: "+win.sensors().get(SoftwareProcess.ADDRESS);
         assertPhaseStreamSatisfies(win, "install", "stdout", Predicates.equalTo(in));
@@ -365,26 +365,28 @@
     public void testDifferentLogLevels() throws Exception {
         List<String> yaml = Lists.newArrayList();
         yaml.addAll(yamlLocation);
-        String hostMsg = "Log in Host Stream";
-        String outputMsg = "Log in Success Stream";
-        String errMsg = "Log in Error Stream";
-        String warningMsg = "Log in Warning Stream";
-        String verboseMsg = "Log in Verbose Stream";
-        String debugMsg = "Log in Debug Stream";
+
+        String hostMsg = "Host Message";
+        String progressMsg = "Progress Message";
+        String outputMsg = "Output Message";
+        String errMsg = "Error Message";
+        String warningMsg = "Warning Message";
+        String verboseMsg = "Verbose Message";
+        String debugMsg = "Debug Message";
+        String informationMsg = "Information Message";
+
         String cmd =
                 "$DebugPreference = \"Continue\"\n" +
                 "$VerbosePreference = \"Continue\"\n" +
                 "\n" +
-                "Write-Host \""+hostMsg+"\"\n" +
-                "Write-Output \""+outputMsg+"\"\n" +
+                "Write-Host \"" + hostMsg + "\"\n" +
+                "Write-Output \"" + outputMsg + "\"\n" +
+                "Write-Progress \"" + progressMsg + "\"\n" +
                 "Write-Error \"" + errMsg + "\" \n" +
                 "Write-Warning \"" + warningMsg + "\"\n" +
                 "Write-Verbose \"" + verboseMsg + "\"\n" +
                 "Write-Debug \"" + debugMsg + "\" \n" +
-
-                        // not always supported:
-//                "Write-Information \"Log in Information Stream\" \n" +
-
+                // "Write-Information \"" + informationMsg + "\" \n" + // Write-Host is a wrapper for Write-Information since PowerShell 5.0
                 "";
 
         yaml.addAll(ImmutableList.of(
@@ -402,86 +404,23 @@
 
 
         Entity win = Iterables.getOnlyElement(app.getChildren());
-        WrappedStream stderr = getWrappedStream(win, "install", "stderr");
-        boolean errStreamContainsStdin = stderr.streamContents.get().contains("Write-Host");
 
-        assertPhaseStreamSatisfies(win, "install", "stdout", s -> s.trim().equalsIgnoreCase(hostMsg+"\n"+outputMsg));
+        // Verify stdout output.
+        assertPhaseStreamSatisfies(win, "install", "stdout", s ->
+                s.trim().equalsIgnoreCase(hostMsg+"\n"+outputMsg));
 
-        // would be nice if we could get host and output in xml out -- but we don't
-        int hostCount = 0+(errStreamContainsStdin ? 1 : 0);
-        assertPhaseStreamSatisfies(win, "install", "stderr", s ->
-                // all but stdout streams included here
-                Strings.countOccurrences(s, errMsg) == 1+(errStreamContainsStdin ? 1 : 0) &&
-                Strings.countOccurrences(s, warningMsg) == 1+(errStreamContainsStdin ? 1 : 0) &&
-                Strings.countOccurrences(s, verboseMsg) == 1+(errStreamContainsStdin ? 1 : 0) &&
-                Strings.countOccurrences(s, debugMsg) == 1+(errStreamContainsStdin ? 1 : 0) &&
-                // not these
-                Strings.countOccurrences(s, hostMsg) == hostCount &&
-                Strings.countOccurrences(s, outputMsg) == hostCount &&
-                // but no xml or escaped newlines
-                !s.toLowerCase().contains("<s") &&
-                s.indexOf("_")<0);
-        assertPhaseStreamSatisfies(win, "install", PlainWinRmExecTaskFactory.WINRM_STREAM_XML_STDERR, s ->
-                // sas above
-                Strings.countOccurrences(s, errMsg) == 1+(errStreamContainsStdin ? 1 : 0) &&
-                Strings.countOccurrences(s, warningMsg) == 1+(errStreamContainsStdin ? 1 : 0) &&
-                Strings.countOccurrences(s, verboseMsg) == 1+(errStreamContainsStdin ? 1 : 0) &&
-                Strings.countOccurrences(s, debugMsg) == 1+(errStreamContainsStdin ? 1 : 0) &&
-                // but actually host and output aren't included ?
-                Strings.countOccurrences(s, hostMsg) == hostCount &&
-                Strings.countOccurrences(s, outputMsg) == hostCount &&
-                // but YES it has XML (and usually escaped new lines but don't test that)
-//                s.indexOf("_")>=0 &&
-                s.toLowerCase().contains("<s s=\"verbose\""));
-
-        assertPhaseStreamSatisfies(win, "install", PlainWinRmExecTaskFactory.WINRM_STREAM_ERROR, s ->
-                // should contain just errors
-                Strings.countOccurrences(s, errMsg) == 1+(errStreamContainsStdin ? 1 : 0) &&
-                // not these
-                Strings.countOccurrences(s, warningMsg) == 0+(errStreamContainsStdin ? 1 : 0) &&
-                Strings.countOccurrences(s, verboseMsg) == 0+(errStreamContainsStdin ? 1 : 0) &&
-                Strings.countOccurrences(s, debugMsg) == 0+(errStreamContainsStdin ? 1 : 0) &&
-                Strings.countOccurrences(s, hostMsg) == 0+(errStreamContainsStdin ? 1 : 0) &&
-                Strings.countOccurrences(s, outputMsg) == 0+(errStreamContainsStdin ? 1 : 0) &&
-                // no xml or escaped newlines
-                !s.toLowerCase().contains("<s") &&
-                s.indexOf("_")<0);
-        assertPhaseStreamSatisfies(win, "install", PlainWinRmExecTaskFactory.WINRM_STREAM_WARNING, s ->
-                // should contain just warning
-                Strings.countOccurrences(s, warningMsg) == 1 &&
-                // not these
-                Strings.countOccurrences(s, errMsg) == 0 &&
-                Strings.countOccurrences(s, verboseMsg) == 0 &&
-                Strings.countOccurrences(s, debugMsg) == 0 &&
-                Strings.countOccurrences(s, hostMsg) == 0 &&
-                Strings.countOccurrences(s, outputMsg) == 0 &&
-                // no xml or escaped newlines
-                !s.toLowerCase().contains("<s") &&
-                s.indexOf("_")<0);
-        assertPhaseStreamSatisfies(win, "install", PlainWinRmExecTaskFactory.WINRM_STREAM_DEBUG, s ->
-                // should contain just warning
-                Strings.countOccurrences(s, debugMsg) == 1 &&
-                // not these
-                Strings.countOccurrences(s, errMsg) == 0 &&
-                Strings.countOccurrences(s, verboseMsg) == 0 &&
-                Strings.countOccurrences(s, warningMsg) == 0 &&
-                Strings.countOccurrences(s, hostMsg) == 0 &&
-                Strings.countOccurrences(s, outputMsg) == 0 &&
-                // no xml or escaped newlines
-                !s.toLowerCase().contains("<s") &&
-                s.indexOf("_")<0);
-        assertPhaseStreamSatisfies(win, "install", PlainWinRmExecTaskFactory.WINRM_STREAM_VERBOSE, s ->
-                // should contain just warning
-                Strings.countOccurrences(s, verboseMsg) == 1 &&
-                // not these
-                Strings.countOccurrences(s, errMsg) == 0 &&
-                Strings.countOccurrences(s, warningMsg) == 0 &&
-                Strings.countOccurrences(s, debugMsg) == 0 &&
-                Strings.countOccurrences(s, hostMsg) == 0 &&
-                Strings.countOccurrences(s, outputMsg) == 0 &&
-                // no xml or escaped newlines
-                !s.toLowerCase().contains("<s") &&
-                s.indexOf("_")<0);
+        // Verify WinRM output to contain error, warning, verbose and debug messages in CLI XML format.
+        assertPhaseStreamSatisfies(win, "install", PlainWinRmExecTaskFactory.WINRM_STREAM, s ->
+                Strings.countOccurrences(s, errMsg + "_x000D__x000A_</S>") == 1 && // error always includes \r\n markers
+                Strings.countOccurrences(s, warningMsg + "</S>") == 1 &&
+                Strings.countOccurrences(s, verboseMsg + "</S>") == 1 &&
+                Strings.countOccurrences(s, debugMsg + "</S>") == 1 &&
+                Strings.countOccurrences(s, "<Obj S=\"progress\"") == 1 && // Progress is a composite object;
+                Strings.countOccurrences(s, progressMsg + "</AV>") == 1 && // message is wrapped with 'AV' tag.
+                // host and output are not included
+                Strings.countOccurrences(s, hostMsg + "</S>") == 0 &&
+                Strings.countOccurrences(s, outputMsg + "</S>") == 0 &&
+                Strings.countOccurrences(s, informationMsg + "</S>") == 0);
     }
     
     private void assertPhaseStreamSatisfies(Entity entity, String phase, String stream, Predicate<String> check) {
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/BrooklynGarbageCollector.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/BrooklynGarbageCollector.java
index 48ead9d..a48f1f0 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/BrooklynGarbageCollector.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/BrooklynGarbageCollector.java
@@ -278,6 +278,7 @@
     
     public void deleteTasksForEntity(Entity entity) {
         // remove all references to this entity from tasks
+        // (note that cancellation for most tasks will have been done by LocalEntityManager.stopTasks)
         executionManager.deleteTag(entity);
         executionManager.deleteTag(BrooklynTaskTags.tagForContextEntity(entity));
         executionManager.deleteTag(BrooklynTaskTags.tagForCallerEntity(entity));
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EntityManagementSupport.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EntityManagementSupport.java
index d13c737..c293168 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EntityManagementSupport.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EntityManagementSupport.java
@@ -268,6 +268,28 @@
             }
             
             synchronized (this) {
+                if (nonDeploymentManagementContext!=null && nonDeploymentManagementContext.getMode()!=null) {
+                    switch (nonDeploymentManagementContext.getMode()) {
+                        case MANAGEMENT_STARTED:
+                            // normal
+                            break;
+                        case PRE_MANAGEMENT:
+                        case MANAGEMENT_REBINDING:
+                        case MANAGEMENT_STARTING:
+                            // odd, but not a problem
+                            log.warn("Management started invoked on "+entity+" when in unexpected state "+nonDeploymentManagementContext.getMode());
+                            break;
+                        case MANAGEMENT_STOPPING:
+                        case MANAGEMENT_STOPPED:
+                            // problematic
+                            throw new IllegalStateException("Cannot start management of "+entity+" at this time; its management has been told to be "+
+                                    nonDeploymentManagementContext.getMode());
+                    }
+                } else {
+                    // odd - already started
+                    log.warn("Management started invoked on "+entity+" when non-deployment context already cleared");
+                }
+
                 nonDeploymentManagementContext = null;
             }
         } catch (Throwable t) {
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalEntityManager.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalEntityManager.java
index 92abbe4..bd8ea5b 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalEntityManager.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalEntityManager.java
@@ -594,13 +594,25 @@
         // try forcibly interrupting tasks on managed entities
         Collection<Exception> exceptions = MutableSet.of();
         try {
+            boolean inTaskForThisEntity = entity.equals(BrooklynTaskTags.getContextEntity(Tasks.current()));
+            Task rootTask = null;
             Set<Task<?>> tasksCancelled = MutableSet.of();
             for (Task<?> t: managementContext.getExecutionContext(entity).getTasks()) {
-                if (entity.equals(BrooklynTaskTags.getContextEntity(Tasks.current())) && hasTaskAsAncestor(t, Tasks.current())) {
-                    // don't cancel if we are running inside a task on the target entity and
-                    // the task being considered is one we have submitted -- e.g. on "stop" don't cancel ourselves!
-                    // but if our current task is from another entity we probably do want to cancel them (we are probably invoking unmanage)
-                    continue;
+                if (inTaskForThisEntity) {
+                    if (rootTask==null) {
+                        rootTask = getRootTask(Tasks.current());
+                    }
+                    if (Objects.equals(rootTask, getRootTask(t))) {
+                        // don't cancel the task if:
+                        // - the current task is against this entity, and
+                        // - the current task and target task are part of the same root true
+                        // e.g. on "stop" don't cancel ourselves, don't cancel things our ancestors have submitted
+                        // (direct ancestry check is not good enough, because we might be in a subtask of a deletion which has a DST manager,
+                        // and cancelling the DST manager is almost as bad as cancelling ourselves);
+                        // however if our current task is from another entity we maybe do want to cancel other things running at this node
+                        // (although maybe not; we could remote the "inTaskForThisEntity" check)
+                        continue;
+                    }
                 }
                 
                 if (!t.isDone()) {
@@ -650,6 +662,15 @@
         return hasTaskAsAncestor(t.getSubmittedByTask(), potentialAncestor);
     }
 
+    private Task<?> getRootTask(Task<?> t) {
+        Task<?> result = t;
+        while (t!=null) {
+            result = t;
+            t = t.getSubmittedByTask();
+        }
+        return result;
+    }
+
     /**
      * activates management when effector invoked, warning unless context is acceptable
      * (currently only acceptable context is "start")
diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/ClockerDynamicLocationPatternRebindTest.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/OnTheFlyDynamicLocationPatternRebindTest.java
similarity index 94%
rename from software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/ClockerDynamicLocationPatternRebindTest.java
rename to software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/OnTheFlyDynamicLocationPatternRebindTest.java
index b190f0c..e68d7ad 100644
--- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/ClockerDynamicLocationPatternRebindTest.java
+++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/OnTheFlyDynamicLocationPatternRebindTest.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.brooklyn.core.location.dynamic.clocker;
+package org.apache.brooklyn.core.location.dynamic.onthefly;
 
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertFalse;
@@ -44,9 +44,11 @@
 import com.google.common.collect.Maps;
 
 /**
- * See explanation of what we're testing in {@link StubInfrastructure}.
+ * Allow creation of a location as part of entities then use of it -- useful for Clocker, and other places too.
+ *
+ * See fuller explanation of what we're testing in {@link StubInfrastructure}.
  */
-public class ClockerDynamicLocationPatternRebindTest extends RebindTestFixtureWithApp {
+public class OnTheFlyDynamicLocationPatternRebindTest extends RebindTestFixtureWithApp {
 
     @Override
     protected LocalManagementContext createOrigManagementContext() {
diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/ClockerDynamicLocationPatternTest.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/OnTheFlyDynamicLocationPatternTest.java
similarity index 96%
rename from software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/ClockerDynamicLocationPatternTest.java
rename to software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/OnTheFlyDynamicLocationPatternTest.java
index 77dd0b1..1306002 100644
--- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/ClockerDynamicLocationPatternTest.java
+++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/OnTheFlyDynamicLocationPatternTest.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.brooklyn.core.location.dynamic.clocker;
+package org.apache.brooklyn.core.location.dynamic.onthefly;
 
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertFalse;
@@ -45,9 +45,9 @@
 /**
  * See explanation of what we're testing in {@link StubInfrastructure}.
  */
-public class ClockerDynamicLocationPatternTest extends BrooklynAppUnitTestSupport {
+public class OnTheFlyDynamicLocationPatternTest extends BrooklynAppUnitTestSupport {
 
-    private static final Logger LOG = LoggerFactory.getLogger(ClockerDynamicLocationPatternTest.class);
+    private static final Logger LOG = LoggerFactory.getLogger(OnTheFlyDynamicLocationPatternTest.class);
 
     private LocalhostMachineProvisioningLocation loc;
 
diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubAttributes.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubAttributes.java
similarity index 95%
rename from software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubAttributes.java
rename to software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubAttributes.java
index e356801..bbae4e9 100644
--- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubAttributes.java
+++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubAttributes.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.brooklyn.core.location.dynamic.clocker;
+package org.apache.brooklyn.core.location.dynamic.onthefly;
 
 import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.sensor.AttributeSensorAndConfigKey;
diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubContainer.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubContainer.java
similarity index 95%
rename from software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubContainer.java
rename to software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubContainer.java
index 504159d..d0d2119 100644
--- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubContainer.java
+++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubContainer.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.brooklyn.core.location.dynamic.clocker;
+package org.apache.brooklyn.core.location.dynamic.onthefly;
 
 import org.apache.brooklyn.api.entity.ImplementedBy;
 import org.apache.brooklyn.core.location.dynamic.LocationOwner;
diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubContainerImpl.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubContainerImpl.java
similarity index 98%
rename from software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubContainerImpl.java
rename to software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubContainerImpl.java
index db5bddf..e2021df 100644
--- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubContainerImpl.java
+++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubContainerImpl.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.brooklyn.core.location.dynamic.clocker;
+package org.apache.brooklyn.core.location.dynamic.onthefly;
 
 import java.io.IOException;
 import java.util.Collection;
diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubContainerLocation.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubContainerLocation.java
similarity index 96%
rename from software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubContainerLocation.java
rename to software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubContainerLocation.java
index 31c3a40..da5be3b 100644
--- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubContainerLocation.java
+++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubContainerLocation.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.brooklyn.core.location.dynamic.clocker;
+package org.apache.brooklyn.core.location.dynamic.onthefly;
 
 import org.apache.brooklyn.api.location.LocationDefinition;
 import org.apache.brooklyn.core.location.dynamic.DynamicLocation;
diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubHost.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubHost.java
similarity index 96%
rename from software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubHost.java
rename to software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubHost.java
index 6b2d319..4eb9885 100644
--- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubHost.java
+++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubHost.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.brooklyn.core.location.dynamic.clocker;
+package org.apache.brooklyn.core.location.dynamic.onthefly;
 
 import org.apache.brooklyn.api.entity.ImplementedBy;
 import org.apache.brooklyn.api.sensor.AttributeSensor;
diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubHostImpl.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubHostImpl.java
similarity index 98%
rename from software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubHostImpl.java
rename to software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubHostImpl.java
index 7cfe822..c70b3eb 100644
--- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubHostImpl.java
+++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubHostImpl.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.brooklyn.core.location.dynamic.clocker;
+package org.apache.brooklyn.core.location.dynamic.onthefly;
 
 import java.util.Map;
 import org.apache.brooklyn.api.entity.EntitySpec;
diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubHostLocation.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubHostLocation.java
similarity index 98%
rename from software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubHostLocation.java
rename to software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubHostLocation.java
index 988bb2e..4f9cc85 100644
--- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubHostLocation.java
+++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubHostLocation.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.brooklyn.core.location.dynamic.clocker;
+package org.apache.brooklyn.core.location.dynamic.onthefly;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubInfrastructure.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubInfrastructure.java
similarity index 98%
rename from software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubInfrastructure.java
rename to software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubInfrastructure.java
index fe060cb..80575ae 100644
--- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubInfrastructure.java
+++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubInfrastructure.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.brooklyn.core.location.dynamic.clocker;
+package org.apache.brooklyn.core.location.dynamic.onthefly;
 
 import java.util.List;
 
diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubInfrastructureImpl.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubInfrastructureImpl.java
similarity index 99%
rename from software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubInfrastructureImpl.java
rename to software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubInfrastructureImpl.java
index 05367c4..f5535da 100644
--- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubInfrastructureImpl.java
+++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubInfrastructureImpl.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.brooklyn.core.location.dynamic.clocker;
+package org.apache.brooklyn.core.location.dynamic.onthefly;
 
 import java.util.Collection;
 import java.util.List;
diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubInfrastructureLocation.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubInfrastructureLocation.java
similarity index 98%
rename from software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubInfrastructureLocation.java
rename to software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubInfrastructureLocation.java
index aaa7d30..71dbec6 100644
--- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubInfrastructureLocation.java
+++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubInfrastructureLocation.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.brooklyn.core.location.dynamic.clocker;
+package org.apache.brooklyn.core.location.dynamic.onthefly;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubResolver.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubResolver.java
similarity index 98%
rename from software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubResolver.java
rename to software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubResolver.java
index 8aaf4a8..952b506 100644
--- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubResolver.java
+++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubResolver.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.brooklyn.core.location.dynamic.clocker;
+package org.apache.brooklyn.core.location.dynamic.onthefly;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubResolverTest.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubResolverTest.java
similarity index 98%
rename from software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubResolverTest.java
rename to software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubResolverTest.java
index 6ffb387..cfdb813 100644
--- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubResolverTest.java
+++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubResolverTest.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.brooklyn.core.location.dynamic.clocker;
+package org.apache.brooklyn.core.location.dynamic.onthefly;
 
 import static org.testng.Assert.assertEquals;
 
diff --git a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubUtils.java b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubUtils.java
similarity index 98%
rename from software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubUtils.java
rename to software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubUtils.java
index eaea4fc..1e1f46e 100644
--- a/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/clocker/StubUtils.java
+++ b/software/base/src/test/java/org/apache/brooklyn/core/location/dynamic/onthefly/StubUtils.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.brooklyn.core.location.dynamic.clocker;
+package org.apache.brooklyn.core.location.dynamic.onthefly;
 
 import javax.annotation.Nullable;
 
diff --git a/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/PlainWinRmExecTaskFactory.java b/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/PlainWinRmExecTaskFactory.java
index 4abaa31..3b0bdce 100644
--- a/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/PlainWinRmExecTaskFactory.java
+++ b/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/PlainWinRmExecTaskFactory.java
@@ -20,52 +20,32 @@
 
 import com.google.common.annotations.Beta;
 import com.google.common.base.Function;
-import com.google.common.collect.Iterables;
 import java.io.ByteArrayOutputStream;
-import java.io.OutputStream;
 import java.io.OutputStreamWriter;
-import java.util.List;
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
-import org.apache.brooklyn.util.collections.MutableList;
-import org.apache.brooklyn.util.core.internal.winrm.winrm4j.FilteringXmlWriter.SelectedStreamsFilteringXmlWriter;
 import org.apache.brooklyn.util.core.internal.winrm.winrm4j.PrettyXmlWriter;
 import org.apache.brooklyn.util.core.task.TaskBuilder;
 import org.apache.brooklyn.util.core.task.ssh.internal.AbstractSshExecTaskFactory;
 import org.apache.brooklyn.util.core.task.ssh.internal.PlainSshExecTaskFactory;
 import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper;
-import org.apache.commons.io.output.TeeOutputStream;
 import org.apache.commons.io.output.WriterOutputStream;
 
 public class PlainWinRmExecTaskFactory<RET> extends AbstractSshExecTaskFactory<PlainSshExecTaskFactory<RET>,RET> {
 
-    public static final String WINRM_STREAM_XML_STDERR = "winrm_xml_stderr";
+    /** The XML-formatted Windows diagnostic stream ID, includes error, warning, debug and verbose messages */
+    public static final String WINRM_STREAM = "winrm";
 
-    public static final String WINRM_STREAM_ERROR = "winrm_error";
-    public static final String WINRM_STREAM_WARNING = "winrm_warning";
-    public static final String WINRM_STREAM_DEBUG = "winrm_debug";
-    public static final String WINRM_STREAM_VERBOSE = "winrm_verbose";
-
-    /** constructor where machine will be added later */
+    /** Constructor where machine will be added later */
     public PlainWinRmExecTaskFactory(String ...commands) {
         super(commands);
     }
 
-    /** convenience constructor to supply machine immediately */
+    /** Constructor to supply machine immediately */
     public PlainWinRmExecTaskFactory(WinRmMachineLocation machine, String ...commands) {
         this(commands);
         machine(machine);
     }
 
-    /** Constructor where machine will be added later */
-    public PlainWinRmExecTaskFactory(List<String> commands) {
-        this(commands.toArray(new String[commands.size()]));
-    }
-
-    /** Convenience constructor to supply machine immediately */
-    public PlainWinRmExecTaskFactory(WinRmMachineLocation machine, List<String> commands) {
-        this(machine, commands.toArray(new String[commands.size()]));
-    }
-
     @Override
     public <T2> PlainWinRmExecTaskFactory<T2> returning(ScriptReturnType type) {
         return (PlainWinRmExecTaskFactory<T2>) super.<T2>returning(type);
@@ -100,43 +80,17 @@
 
     @Beta
     public static Std2x2StreamProvider newStreamProviderForWindowsXml(TaskBuilder<?> tb) {
-        Std2x2StreamProvider r = new Std2x2StreamProvider();
-        r.stdoutForWriting = r.stdoutForReading = new ByteArrayOutputStream();
-        tb.tag(BrooklynTaskTags.tagForStreamSoft(BrooklynTaskTags.STREAM_STDOUT, r.stdoutForReading));
+        Std2x2StreamProvider std2x2StreamProvider = new Std2x2StreamProvider();
+        std2x2StreamProvider.stdoutForWriting = std2x2StreamProvider.stdoutForReading = new ByteArrayOutputStream();
+
+        tb.tag(BrooklynTaskTags.tagForStreamSoft(BrooklynTaskTags.STREAM_STDOUT, std2x2StreamProvider.stdoutForReading));
 
         ByteArrayOutputStream stderrXmlPrettyOut = new ByteArrayOutputStream();
-        ByteArrayOutputStream stderrNonXmlFilteredOut = new ByteArrayOutputStream();
-        ByteArrayOutputStream verboseFilteredOut = new ByteArrayOutputStream();
-        ByteArrayOutputStream debugFilteredOut = new ByteArrayOutputStream();
-        ByteArrayOutputStream warningFilteredOut = new ByteArrayOutputStream();
-        ByteArrayOutputStream errorFilteredOut = new ByteArrayOutputStream();
+        std2x2StreamProvider.stderrForWriting = new WriterOutputStream(new PrettyXmlWriter(new OutputStreamWriter(stderrXmlPrettyOut)));
 
-        OutputStream rawxml = tee(MutableList.of(
-                new WriterOutputStream(new PrettyXmlWriter(new OutputStreamWriter(stderrXmlPrettyOut))),
-                new WriterOutputStream(new SelectedStreamsFilteringXmlWriter(s -> true, stderrNonXmlFilteredOut)),
-                new WriterOutputStream(new SelectedStreamsFilteringXmlWriter("verbose", verboseFilteredOut)),
-                new WriterOutputStream(new SelectedStreamsFilteringXmlWriter("debug", debugFilteredOut)),
-                new WriterOutputStream(new SelectedStreamsFilteringXmlWriter("warning", warningFilteredOut)),
-                new WriterOutputStream(new SelectedStreamsFilteringXmlWriter("error", errorFilteredOut)) ));
+        tb.tag(BrooklynTaskTags.tagForStreamSoft(WINRM_STREAM, stderrXmlPrettyOut));
 
-        tb.tag(BrooklynTaskTags.tagForStreamSoft(BrooklynTaskTags.STREAM_STDERR, stderrNonXmlFilteredOut));
-
-        tb.tag(BrooklynTaskTags.tagForStreamSoft(WINRM_STREAM_XML_STDERR, stderrXmlPrettyOut));
-        tb.tag(BrooklynTaskTags.tagForStreamSoft(WINRM_STREAM_VERBOSE, verboseFilteredOut));
-        tb.tag(BrooklynTaskTags.tagForStreamSoft(WINRM_STREAM_DEBUG, debugFilteredOut));
-        tb.tag(BrooklynTaskTags.tagForStreamSoft(WINRM_STREAM_WARNING, warningFilteredOut));
-        tb.tag(BrooklynTaskTags.tagForStreamSoft(WINRM_STREAM_ERROR, errorFilteredOut));
-
-        r.stderrForReading = stderrNonXmlFilteredOut;
-        r.stderrForWriting = rawxml;
-
-        return r;
-    }
-
-    private static OutputStream tee(List<OutputStream> streams) {
-        if (streams==null || streams.isEmpty()) throw new IllegalStateException("stream required");
-        if (streams.size()==1) return Iterables.getOnlyElement(streams);
-        return new TeeOutputStream(streams.get(0), tee(streams.subList(1, streams.size())));
+        return std2x2StreamProvider;
     }
 }
 
diff --git a/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/FilteringXmlWriter.java b/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/FilteringXmlWriter.java
deleted file mode 100644
index be075a5..0000000
--- a/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/FilteringXmlWriter.java
+++ /dev/null
@@ -1,335 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.brooklyn.util.core.internal.winrm.winrm4j;
-
-import com.google.common.collect.ImmutableSet;
-import java.io.*;
-import java.util.Set;
-import java.util.function.Predicate;
-import org.apache.brooklyn.util.collections.MutableSet;
-import org.apache.brooklyn.util.exceptions.Exceptions;
-import org.apache.brooklyn.util.text.StringEscapes;
-import org.apache.brooklyn.util.text.Strings;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-/** Filters XML output in a configurable way, such as subclasses
- *  ErrorFiltering for any S tag with an S attribute of "error" and extracts the text, one per line.
- * Or if any error or not XML, switches to pass through.
- *
- * Supports streaming, so viewers get output as quickly as possible. Accepts # comment lines.
- * But otherwise very rough-and-ready! */
-public class FilteringXmlWriter extends Writer {
-
-    private static final Logger LOG = LoggerFactory.getLogger(FilteringXmlWriter.class);
-
-    public static class SelectedStreamsFilteringXmlWriter extends FilteringXmlWriter {
-        public final Predicate<String> allowStreams;
-        public SelectedStreamsFilteringXmlWriter(Predicate<String> allowStreams, Writer writer) {
-            super(writer);
-            this.allowStreams = allowStreams;
-        }
-        public SelectedStreamsFilteringXmlWriter(String stream, Writer writer) {
-            this(ImmutableSet.of(stream), writer);
-        }
-        public SelectedStreamsFilteringXmlWriter(Set<String> streams, Writer writer) {
-            this(s -> streams.contains(s), writer);
-        }
-
-        public SelectedStreamsFilteringXmlWriter(String stream, OutputStream out) {
-            this(stream, new OutputStreamWriter(out));
-        }
-        public SelectedStreamsFilteringXmlWriter(Predicate<String> allowStreams, OutputStream out) {
-            this(allowStreams, new OutputStreamWriter(out));
-        }
-
-        protected void processAttributeValue(String attribute, String value, String tag) {
-            if ("S".equalsIgnoreCase(tag) && "S".equalsIgnoreCase(attribute)
-                && allowStreams.test(
-                        StringEscapes.BashStringEscapes.unwrapBashQuotesAndEscapes(value.trim()).toLowerCase())
-            ) {
-                allowTextHere();
-            }
-        }
-    }
-
-    private final Writer wrappedWriter;
-
-    // means we give up, probably not xml, just echo
-    private boolean passThrough;
-    private boolean waitingForContent = true;
-
-    private boolean inTag;
-
-    private boolean inTagName;
-    private String tag;
-    private boolean thisTagIsAnEndTag;
-    private boolean thisTagIsSelfClosing;
-
-    private boolean inAttribute;
-    private String attribute;
-
-    private boolean inValue;
-    private String value;
-
-    private boolean inComment;
-
-    private String textWrittenHere;
-    private boolean textAllowedHere;
-
-    private boolean isLastCharBackslashEscaped;
-    private boolean isLastCharLineStart;
-    private boolean cacheLastCharLineStart;
-
-    public FilteringXmlWriter(Writer writer) {
-        super(writer);
-        wrappedWriter = writer;
-    }
-
-    @Override
-    public void write(char[] cbuf, int off, int len) throws IOException {
-        char c;
-        cacheLastCharLineStart = true;
-        for (int i = off; i < off + len; i++) {
-            isLastCharLineStart = cacheLastCharLineStart;
-            c = cbuf[i];
-            cacheLastCharLineStart = (c=='\n');
-
-            if (passThrough) {
-                writeChar(c);
-                continue;
-            }
-
-            try {
-                if (inComment) {
-                    // Currently in a comment (weird MS comment not xml comment)
-                    if (c == '\n') {
-                        inComment = false;
-                    }
-                    continue;
-                }
-
-                if (isLastCharLineStart && c == '#') {
-                    inComment = true;
-                    continue;
-                }
-
-                if (inTag) {
-                    if (inTagName) {
-                        if (c == ' ') {
-                            if (!tag.isEmpty()) {
-                                onTagBegin(tag);
-                                inAttribute = true;
-                                attribute = "";
-                            } else {
-                                // ignore
-                            }
-                        } else if (c == '>') {
-                            onTagBegin(tag);
-                            processTagFinished(tag);
-                        } else if (c == '/') {
-                            // nothing
-                            if (tag.isEmpty()) {
-                                thisTagIsAnEndTag = true;
-                            } else {
-                                onTagBegin(tag);
-                            }
-
-                        } else {
-                            tag = tag + c;
-                        }
-                        continue;
-                    }
-
-
-                    if (inAttribute) {
-                        if (c == '=') {
-                            inAttribute = false;
-                            inValue = true;
-                            value = "";
-                            continue;
-                        } else if (c == ' ') {
-                            if (Strings.isBlank(attribute)) {
-                                // skip
-                            } else {
-                                inAttribute = false;
-                            }
-                        } else if (c == '>') {
-                            inAttribute = false;
-                            inTag = false;
-                        } else if (c == '/') {
-                            // nothing
-                        } else {
-                            attribute = attribute + c;
-                        }
-                        continue;
-                    }
-
-                    if (inValue) {
-                        if (c == '\\') {
-                            isLastCharBackslashEscaped = true;
-                            continue;
-                        }
-                        if (isLastCharBackslashEscaped) {
-                            isLastCharBackslashEscaped = false;
-                            value += "\\" + c;
-                            continue;
-                        }
-                        if (c == '"') {
-                            if (value.trim().isEmpty()) {
-                                // string start
-                                value += c;
-                                continue;
-                            } else {
-                                // string end
-                                value += c;
-                                processAttributeValue(attribute, value, tag);
-                                attribute = null;
-                                value = null;
-                                inValue = false;
-                                continue;
-                            }
-                        }
-                        value = value + c;
-                        continue;
-                    }
-
-                    if (c == '=') {
-                        inValue = true;
-                        value = "";
-                        continue;
-                    }
-
-                    if (c == '\n') {
-                        continue;
-                    }
-
-                    if (c == ' ') {
-                        continue;
-                    }
-
-                    if (c == '>') {
-                        processTagFinished(tag);
-                        continue;
-                    }
-
-                    // otherwise it starts a new attribute
-                    inAttribute = true;
-                    attribute = "";
-
-                    continue;
-                }
-
-                if (c == '<') {
-                    waitingForContent = false;
-                    thisTagIsSelfClosing = false;
-                    thisTagIsAnEndTag = false;
-                    inTag = true;
-                    inTagName = true;
-                    thisTagIsAnEndTag = false;
-                    tag = "";
-                    endTextAllowed();
-                    continue;
-                }
-
-                if (textAllowedHere) {
-                    // could add to textWrittenHere here
-                    writeChar(c);
-                }
-
-                if (waitingForContent && !Character.isWhitespace(c)) {
-                    // not xml
-                    writeChar(c);
-                    passThrough = true;
-                    continue;
-                }
-
-            } catch (Exception e) {
-                Exceptions.propagateIfFatal(e);
-                LOG.debug("Error trying to process WinRM output as XML; switching to pass-through: "+e, e);
-                passThrough = true;
-            }
-        }
-    }
-
-    protected void endTextAllowed() throws IOException {
-        if (textAllowedHere && !isLastCharLineStart) {
-            writeChar('\n');
-        }
-        textWrittenHere = null;
-        textAllowedHere = false;
-    }
-
-    protected void onTagBegin(String tag) {
-        // might be start or end tag, we don't track that.  we don't need this at all actually currently but maybe one day
-//        LOG.info(tag+":");
-        inTagName = false;
-    }
-
-    protected void processAttributeValue(String attribute, String value, String tag) {
-//        LOG.info(tag+" @"+attribute+ " = "+value);
-    }
-
-    protected void allowTextHere() {
-        textAllowedHere = true;
-        textWrittenHere = "";
-    }
-
-    protected void processTagFinished(String tag) {
-//        if (thisTagIsAnEndTag) LOG.info("/"+tag);
-//        else if (thisTagIsSelfClosing) LOG.info("  "+tag+"/");
-//        else LOG.info("  .");
-        inTag = false;
-    }
-
-    String buffered = null;
-    protected void writeChar(char c) throws IOException {
-        if (buffered !=null) {
-            buffered += c;
-            if (c=='_') {
-                if ("_x000A_".equalsIgnoreCase(buffered)) {
-                    wrappedWriter.write('\n');
-                    cacheLastCharLineStart = true;
-                } else if ("_x000D_".equalsIgnoreCase(buffered)) {
-                    // suppress
-                } else {
-                    wrappedWriter.write(buffered);
-                }
-                buffered = null;
-            }
-            return;
-        }
-        if (c=='_') {
-            buffered = ""+c;
-            return;
-        }
-
-        wrappedWriter.write(c);
-    }
-
-    @Override
-    public void flush() throws IOException {
-        wrappedWriter.flush();
-    }
-
-    @Override
-    public void close() throws IOException {
-        wrappedWriter.close();
-    }
-}
diff --git a/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/PrettyXmlWriter.java b/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/PrettyXmlWriter.java
index 70b5811..531ac09 100644
--- a/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/PrettyXmlWriter.java
+++ b/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/PrettyXmlWriter.java
@@ -23,11 +23,12 @@
 
 public class PrettyXmlWriter extends Writer {
     private final Writer wrappedWriter;
-    private boolean tagClosed = false;
+    private boolean selfClosingTag = false; // ends with '/>'
     private int indentLevel = 0;
     private boolean newLine = true;
     private char lastChar = '\n';
     private boolean isComment = false;
+    private boolean newTagToProcess = false; // new tag '<'
 
     public PrettyXmlWriter(Writer writer) {
         super(writer);
@@ -36,6 +37,14 @@
 
     @Override
     public void write(char[] cbuf, int off, int len) throws IOException {
+
+        // Process new tag '<' from previous call if any.
+        if (newTagToProcess) {
+            handleMeaningfulChar(cbuf, off, '<', off, len);
+            newTagToProcess = false;
+        }
+
+        // Process new data.
         for (int i = off; i < off + len; i++) {
             char c = cbuf[i];
             if (isComment) {
@@ -52,52 +61,60 @@
                 writeChar(c);
             } else {
                 //Not a comment - treat as XML
-                handleMeaningfulChar(cbuf, i, c, off + len - 1);
+                handleMeaningfulChar(cbuf, i, c, off, len);
             }
         }
     }
 
-    private void handleMeaningfulChar(char[] cbuf, int i, char c, int end) throws IOException {
-        if (tagClosed) {
-            // Tag was closed on last call to write so need a new line
-            writeNewLine();
-            tagClosed = false;
+    private void handleMeaningfulChar(char[] cbuf, int i, char c, int off, int len) throws IOException {
+        int end = off + len - 1;
+        boolean endOfChunk = end == i;
+
+        // Reduce indentation level after closing a tag, starts with '</'
+        if (lastChar == '<' && c == '/' || newTagToProcess && cbuf[i] == '/') {
+            indentLevel--;
         }
+
+        // Mark self-closing tag, ends with '/>'
+        if (lastChar == '/' && c == '>') {
+            selfClosingTag = true;
+        }
+
+        // Process a new tag, starts with '<'
         if (c == '<') {
-            // Start if a tag
-            if (!newLine) {
-                // Assume closing tag following text - e.g. <t>some text</t>
-                indentLevel--;
-            } else if (i < end && cbuf[i + 1] == '/') {
-                // Closing tag
-                indentLevel--;
-                printIndent();
-                indentLevel--;
-            } else {
-                //
-                printIndent();
-            }
-        }
-        writeChar(c);
-        if ('>' == c ) {
-            if (i < end) {
-                if (cbuf[i + 1] == '<') {
-                    writeNewLine();
-                    increaseIndentIfNotSelfClosingTag();
-                }
-            } else {
-                // We've closed a tag but don't know what the next character is
-                tagClosed = true;
-                increaseIndentIfNotSelfClosingTag();
-            }
-        }
-        lastChar = c;
-    }
 
-    private void increaseIndentIfNotSelfClosingTag() {
-        if (lastChar != '/') {
-            indentLevel++;
+            // Process adjacent tags (any), '><'
+            if ('>' == lastChar) {
+
+                // Process remaining new tag '<' in the next call, if we hit the end of current chunk.
+                if (endOfChunk) {
+                    newTagToProcess = true;
+                    return;
+                }
+
+                // Write new line between adjacent tags '>\n<'
+                writeNewLine();
+
+                // Increase indentation level, assume nested content
+                indentLevel++;
+
+                if (i < end && cbuf[i + 1] == '/') {
+                    indentLevel--;
+                }
+
+                // Reduce indentation level for self-closing tag '/>'
+                if (selfClosingTag) {
+                    indentLevel--;
+                    selfClosingTag = false;
+                }
+
+                // Write indentation at a calculated level
+                printIndent();
+            }
         }
+
+        // Write a character.
+        writeChar(c);
     }
 
     private void writeNewLine() throws IOException {
@@ -108,6 +125,7 @@
     private void writeChar(char c) throws IOException {
         wrappedWriter.write(c);
         newLine = false;
+        lastChar = c;
     }
 
     private void printIndent() throws IOException {
diff --git a/software/winrm/src/test/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/ErrorXmlWriterTest.java b/software/winrm/src/test/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/ErrorXmlWriterTest.java
deleted file mode 100644
index 0730b54..0000000
--- a/software/winrm/src/test/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/ErrorXmlWriterTest.java
+++ /dev/null
@@ -1,109 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.brooklyn.util.core.internal.winrm.winrm4j;
-
-import java.io.IOException;
-import org.apache.brooklyn.util.core.internal.winrm.winrm4j.PrettyXmlWriterTest.RecordingWriter;
-import org.testng.Assert;
-import static org.testng.Assert.assertEquals;
-import org.testng.annotations.BeforeMethod;
-import org.testng.annotations.Test;
-
-public class ErrorXmlWriterTest {
-
-    public static final String XML_SMALL = "\n<S S=\"error\">Some error</S>\n#foo \n<S S=\"other\">not-error</S>\n\n";
-    public static final String NOT_XML = "# foo\n\nbar";
-
-    public static final String XML_BIG = "#< CLIXML\n" +
-            "<Objs Version=\"1.1.0.1\" xmlns=\"http://schemas.microsoft.com/powershell/2004/04\">\n" +
-            "\t<Obj S=\"progress\" RefId=\"0\">\n" +
-            "\t\t<TN RefId=\"0\">\n" +
-            "\t\t\t<T>System.Management.Automation.PSCustomObject</T>\n" +
-            "\t\t\t<T>System.Object</T>\n" +
-            "\t\t</TN>\n" +
-            "\t\t<MS>\n" +
-            "\t\t\t<I64 N=\"SourceId\">1</I64>\n" +
-            "\t\t\t<PR N=\"Record\">\n" +
-            "\t\t\t\t<AV>Preparing modules for first use.</AV>\n" +
-            "\t\t\t\t<AI>0</AI>\n" +
-            "\t\t\t\t<Nil />\n" +
-            "\t\t\t\t<PI>-1</PI>\n" +
-            "\t\t\t\t<PC>-1</PC>\n" +
-            "\t\t\t\t<T>Completed</T>\n" +
-            "\t\t\t\t<SR>-1</SR>\n" +
-            "\t\t\t\t<SD> </SD>\n" +
-            "\t\t\t</PR>\n" +
-            "\t\t</MS>\n" +
-            "\t</Obj>\n" +
-            "\t<Obj S=\"progress\" RefId=\"1\">\n" +
-            "\t\t<TNRef RefId=\"0\" />\n" +
-            "\t\t<MS>\n" +
-            "\t\t\t<I64 N=\"SourceId\">1</I64>\n" +
-            "\t\t\t<PR N=\"Record\">\n" +
-            "\t\t\t\t<AV>Preparing modules for first use.</AV>\n" +
-            "\t\t\t\t<AI>0</AI>\n" +
-            "\t\t\t\t<Nil />\n" +
-            "\t\t\t\t<PI>-1</PI>\n" +
-            "\t\t\t\t<PC>-1</PC>\n" +
-            "\t\t\t\t<T>Completed</T>\n" +
-            "\t\t\t\t<SR>-1</SR>\n" +
-            "\t\t\t\t<SD> </SD>\n" +
-            "\t\t\t</PR>\n" +
-            "\t\t</MS>\n" +
-            "\t</Obj>\n" +
-            "\t<S S=\"error\">NEEDLE</S>"+
-            "\t<S S=\"error\">Found\n\n</S>";
-
-    private RecordingWriter recordingWriter;
-    private FilteringXmlWriter xmlWriter;
-
-    @BeforeMethod
-    public void setUp() {
-        recordingWriter = new RecordingWriter();
-        xmlWriter = new FilteringXmlWriter.SelectedStreamsFilteringXmlWriter("error", recordingWriter);
-    }
-
-    @Test
-    public void testSmall() throws IOException {
-        xmlWriter.write(XML_SMALL, 0, XML_SMALL.length());
-        xmlWriter.flush();
-
-        String s = recordingWriter.out.toString();
-        Assert.assertEquals(s, "Some error\n");
-    }
-
-    @Test
-    public void testNotXml() throws IOException {
-        xmlWriter.write(NOT_XML, 0, NOT_XML.length());
-        xmlWriter.flush();
-
-        String s = recordingWriter.out.toString();
-        Assert.assertEquals(s, "bar");
-    }
-
-    @Test
-    public void testBig() throws IOException {
-        xmlWriter.write(XML_BIG, 0, XML_BIG.length());
-        xmlWriter.flush();
-
-        String s = recordingWriter.out.toString();
-        Assert.assertEquals(s, "NEEDLE\nFound\n\n");
-    }
-
-}
\ No newline at end of file
diff --git a/software/winrm/src/test/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/PrettyXmlWriterTest.java b/software/winrm/src/test/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/PrettyXmlWriterTest.java
index 1a6409a..85b24c4 100644
--- a/software/winrm/src/test/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/PrettyXmlWriterTest.java
+++ b/software/winrm/src/test/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/PrettyXmlWriterTest.java
@@ -120,10 +120,10 @@
 
     @Test
     public void testDontIndentSelfClosingTag() throws IOException {
-        String xml = "<t1><t2><t3/></t2></t1>";
+        String xml = "<t1><t2><t3/><t3/></t2><t3/></t1>";
         prettyXmlWriter.write(xml, 0, xml.length());
 
-        assertEquals(recordingWriter.out.toString(), "<t1>\n\t<t2>\n\t\t<t3/>\n\t</t2>\n</t1>");
+        assertEquals(recordingWriter.out.toString(), "<t1>\n\t<t2>\n\t\t<t3/>\n\t\t<t3/>\n\t</t2>\n\t<t3/>\n</t1>");
     }
 
     @Test
@@ -134,6 +134,23 @@
         assertEquals(recordingWriter.out.toString(), "#< CLIXML\n<t1>\n\t<t2>\n\t\t<t3/>\n\t</t2>\n</t1>");
     }
 
+    @Test
+    public void testWriteInChunks() throws IOException {
+        String xml = "<t1><t3/><t2>A</t2><t2>A</t2></t1>";
+        for (int breakAt = 0; breakAt < xml.length(); breakAt++) {
+            recordingWriter = new RecordingWriter();
+            prettyXmlWriter = new PrettyXmlWriter(recordingWriter);
+            System.out.print(xml.substring(0, breakAt) + " ");
+            prettyXmlWriter.write(xml, 0, breakAt);
+            prettyXmlWriter.flush();
+            System.out.println(xml.substring(breakAt));
+            prettyXmlWriter.write(xml, breakAt ,xml.length() - breakAt);
+            prettyXmlWriter.flush();
+            assertEquals(recordingWriter.out.toString(), "<t1>\n\t<t3/>\n\t<t2>A</t2>\n\t<t2>A</t2>\n</t1>",
+                    "Failed to format chunks split at position " + breakAt);
+        }
+    }
+
     private int countNewLines() {
         int newLines = 0;
         String s = recordingWriter.out.toString();