Repository: spring-cloud/spring-cloud-dataflow
Issue: Argument sanitizer regex requires some more fine tuning

The argument sanitizer fails on the following stream DSL:

```
twitterstream --twitter.stream.stream-type=filter --twitter.stream.track=java --twitter.credentials.access-token-secret= --twitter.credentials.consumer-secret= --twitter.credentials.consumer-key=  --twitter.credentials.access-token= | filter --filter.expression=#jsonPath(payload,'$.lang')=='en' | log
```

Changed files:
spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/controller/StreamDefinitionController.java
spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/controller/StreamControllerTests.java
spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/support/ArgumentSanitizerTest.java
spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/controller/StreamDeploymentController.java
spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/controller/support/ArgumentSanitizer.java

diff --git a/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/controller/StreamDefinitionController.java b/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/controller/StreamDefinitionController.java
index 246093909..576c92897 100644
--- a/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/controller/StreamDefinitionController.java
+++ b/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/controller/StreamDefinitionController.java
@@ -247,7 +247,7 @@ public class StreamDefinitionController {
 		@Override
 		public StreamDefinitionResource instantiateResource(StreamDefinition stream) {
 			final StreamDefinitionResource resource = new StreamDefinitionResource(stream.getName(),
-					ArgumentSanitizer.sanitizeStream(stream.getDslText()));
+					new ArgumentSanitizer().sanitizeStream(stream));
 			DeploymentState deploymentState = streamDeploymentStates.get(stream);
 			if (deploymentState != null) {
 				final DeploymentStateResource deploymentStateResource = ControllerUtils
diff --git a/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/controller/StreamDeploymentController.java b/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/controller/StreamDeploymentController.java
index 874362e5b..4e45d3ed2 100644
--- a/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/controller/StreamDeploymentController.java
+++ b/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/controller/StreamDeploymentController.java
@@ -184,7 +184,7 @@ public class StreamDeploymentController {
 				deploymentProperties = streamDeployment.getDeploymentProperties();
 			}
 			return new StreamDeploymentResource(streamDeployment.getStreamName(),
-					ArgumentSanitizer.sanitizeStream(this.dslText), deploymentProperties, this.status);
+					new ArgumentSanitizer().sanitizeStream(new StreamDefinition(streamDeployment.getStreamName(), this.dslText)), deploymentProperties, this.status);
 		}
 
 		private boolean canDisplayDeploymentProperties() {
diff --git a/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/controller/support/ArgumentSanitizer.java b/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/controller/support/ArgumentSanitizer.java
index 3bd7a8bc9..c90098ae9 100644
--- a/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/controller/support/ArgumentSanitizer.java
+++ b/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/controller/support/ArgumentSanitizer.java
@@ -16,11 +16,14 @@
 
 package org.springframework.cloud.dataflow.server.controller.support;
 
-import java.util.regex.Matcher;
+import java.util.List;
+import java.util.Map;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
-import org.springframework.util.Assert;
-import org.springframework.util.StringUtils;
+import org.springframework.cloud.dataflow.core.StreamAppDefinition;
+import org.springframework.cloud.dataflow.core.StreamDefinition;
+import org.springframework.cloud.dataflow.core.StreamDefinitionToDslConverter;
 
 /**
  * Sanitizes potentially sensitive keys for a specific command line arg.
@@ -34,24 +37,17 @@ public class ArgumentSanitizer {
 
 	private static final String[] KEYS_TO_SANITIZE = { "password", "secret", "key", "token", ".*credentials.*",
 			"vcap_services" };
-	//used to find the passwords embedded in a stream definition
-	private static Pattern passwordParameterPatternForStreams = Pattern.compile(
-			//Search for the -- characters then look for unicode letters
-			"(?i)(--[\\\\p{Z}]*[\\\\p{L}.]*[\\\\p{Pd}]*("
-					//that match the following strings from the KEYS_TO_SANITIZE array
-					+ StringUtils.arrayToDelimitedString(KEYS_TO_SANITIZE, "|")
-					//Following the equal sign (group1) accept any number of unicode characters(letters, open punctuation, close punctuation etc) for the value to be sanitized for group 3.
-					+ ")[\\\\p{L}-]*[\\\\p{Z}]*=[\\\\p{Z}]*)((\\"[\\\\p{L}-|\\\\p{Pd}|\\\\p{Ps}|\\\\p{Pe}|\\\\p{Pc}|\\\\p{S}|\\\\p{N}|\\\\p{Z}]*\\")|([\\\\p{N}|\\\\p{L}-|\\\\p{Po}|\\\\p{Pc}|\\\\p{S}]*))",
-			Pattern.UNICODE_CASE);
-
 
 	private Pattern[] keysToSanitize;
 
+	private StreamDefinitionToDslConverter dslConverter;
+
 	public ArgumentSanitizer() {
 		this.keysToSanitize = new Pattern[KEYS_TO_SANITIZE.length];
 		for (int i = 0; i < keysToSanitize.length; i++) {
 			this.keysToSanitize[i] = getPattern(KEYS_TO_SANITIZE[i]);
 		}
+		this.dslConverter = new StreamDefinitionToDslConverter();
 	}
 
 	private Pattern getPattern(String value) {
@@ -107,38 +103,24 @@ public class ArgumentSanitizer {
 	}
 
 	/**
-	 * Redacts sensitive values in a stream.
+	 * Redacts sensitive property values in a stream.
 	 *
-	 * @param definition the definition to sanitize
-	 * @return Stream definition that has sensitive data redacted.
+	 * @param streamDefinition the stream definition to sanitize
+	 * @return Stream definition text that has sensitive data redacted.
 	 */
-	public static String sanitizeStream(String definition) {
-		Assert.hasText(definition, "definition must not be null nor empty");
-		final StringBuffer output = new StringBuffer();
-		final Matcher matcher = passwordParameterPatternForStreams.matcher(definition);
-		while (matcher.find()) {
-			String passwordValue = matcher.group(3);
-
-			String maskedPasswordValue;
-			boolean isPipeAppended = false;
-			boolean isNameChannelAppended = false;
-			if (passwordValue.endsWith("|")) {
-				isPipeAppended = true;
-			}
-			if (passwordValue.endsWith(">")) {
-				isNameChannelAppended = true;
-			}
-			maskedPasswordValue = REDACTION_STRING;
-			if (isPipeAppended) {
-				maskedPasswordValue = maskedPasswordValue + " |";
-			}
-			if (isNameChannelAppended) {
-				maskedPasswordValue = maskedPasswordValue + " >";
-			}
-			matcher.appendReplacement(output, matcher.group(1) + maskedPasswordValue);
-		}
-		matcher.appendTail(output);
-		return output.toString();
+	public String sanitizeStream(StreamDefinition streamDefinition) {
+		List<StreamAppDefinition> sanitizedAppDefinitions = streamDefinition.getAppDefinitions().stream()
+				.map(app -> StreamAppDefinition.Builder
+						.from(app)
+						.setProperties(this.sanitizeProperties(app.getProperties()))
+						.build(streamDefinition.getName())
+				).collect(Collectors.toList());
+
+		return this.dslConverter.toDsl(sanitizedAppDefinitions);
 	}
 
+	private Map<String, String> sanitizeProperties(Map<String, String> properties) {
+		return properties.entrySet().stream()
+				.collect(Collectors.toMap(e -> e.getKey(), e -> this.sanitize(e.getKey(), e.getValue())));
+	}
 }
diff --git a/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/controller/StreamControllerTests.java b/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/controller/StreamControllerTests.java
index b6eb478a9..dc770c517 100644
--- a/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/controller/StreamControllerTests.java
+++ b/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/controller/StreamControllerTests.java
@@ -30,7 +30,6 @@ import org.mockito.ArgumentCaptor;
 
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.context.SpringBootTest;
-import org.springframework.cloud.dataflow.configuration.metadata.ApplicationConfigurationMetadataResolver;
 import org.springframework.cloud.dataflow.core.BindingPropertyKeys;
 import org.springframework.cloud.dataflow.core.StreamAppDefinition;
 import org.springframework.cloud.dataflow.core.StreamDefinition;
@@ -40,7 +39,6 @@ import org.springframework.cloud.dataflow.server.configuration.TestDependencies;
 import org.springframework.cloud.dataflow.server.repository.DeploymentIdRepository;
 import org.springframework.cloud.dataflow.server.repository.DeploymentKey;
 import org.springframework.cloud.dataflow.server.repository.StreamDefinitionRepository;
-import org.springframework.cloud.dataflow.server.service.StreamService;
 import org.springframework.cloud.deployer.resource.maven.MavenResource;
 import org.springframework.cloud.deployer.spi.app.AppDeployer;
 import org.springframework.cloud.deployer.spi.app.AppInstanceStatus;
@@ -95,9 +93,6 @@ public class StreamControllerTests {
 	@Autowired
 	private DeploymentIdRepository deploymentIdRepository;
 
-	@Autowired
-	private ApplicationConfigurationMetadataResolver metadataResolver;
-
 	private MockMvc mockMvc;
 
 	@Autowired
@@ -109,9 +104,6 @@ public class StreamControllerTests {
 	@Autowired
 	private CommonApplicationProperties appsProperties;
 
-	@Autowired
-	private StreamService defaultStreamService;
-
 	@Before
 	public void setupMocks() {
 		this.mockMvc = MockMvcBuilders.webAppContextSetup(wac)
@@ -261,8 +253,8 @@ public class StreamControllerTests {
 		response = mockMvc
 				.perform(get("/streams/definitions/myStream5/related?nested=true").accept(MediaType.APPLICATION_JSON))
 				.andReturn().getResponse().getContentAsString();
-		assertTrue(response.contains(":myStream5.time > log --password=******"));
-		assertTrue(response.contains("time | log --secret=******"));
+		assertTrue(response.contains(":myStream5.time > log --password='******'"));
+		assertTrue(response.contains("time | log --secret='******'"));
 		assertTrue(response.contains("\\"totalElements\\":2"));
 
 		String response2 = mockMvc.perform(
@@ -313,7 +305,7 @@ public class StreamControllerTests {
 				.param("deploy", "false"))
 				.andExpect(status().isCreated());
 		mockMvc.perform(post("/streams/definitions").param("name", "timelogDoubleTick")
-				.param("definition", "time --format=\\"YYYY MM DD\\" | log")
+				.param("definition", "a: time --format=\\"YYYY MM DD\\" | log")
 				.param("deploy", "false")).andExpect(status().isCreated());
 		mockMvc.perform(post("/streams/definitions/").param("name", "twoPassword")
 				.param("definition", "time --password='foo'| log --password=bar")
@@ -332,8 +324,8 @@ public class StreamControllerTests {
 				.perform(get("/streams/definitions/").accept(MediaType.APPLICATION_JSON))
 				.andReturn().getResponse().getContentAsString();
 
-		assertTrue(response.contains("time --password=****** | log"));
-		assertTrue(response.contains("time --foo=bar| log"));
+		assertTrue(response.contains("time --password='******' | log"));
+		assertTrue(response.contains("time --foo=bar | log"));
 
 		assertTrue(response.contains(":myStream1.time > log"));
 		assertTrue(response.contains("time | log"));
@@ -346,19 +338,18 @@ public class StreamControllerTests {
 		assertTrue(response.contains(":myStream2 > log"));
 		assertTrue(response.contains(":myStream3 > log"));
 		assertTrue(response.contains("time --format='YYYY MM DD' | log"));
-		assertTrue(response.contains("time --format=\\\\\\"YYYY MM DD\\\\\\" | log"));
-		assertTrue(response.contains("time --password=****** | log --password=******"));
-		System.out.println(response);
-		assertTrue(response.contains("time --password=****** > :foobar"));
-		assertTrue(response.contains("time --password=****** --arg=foo | log"));
-		assertTrue(response.contains("time --password=****** --arg=bar | log"));
+		assertTrue(response.contains("a: time --format='YYYY MM DD' | log"));
+		assertTrue(response.contains("time --password='******' | log --password='******'"));
+		assertTrue(response.contains("time --password='******' > :foobar"));
+		assertTrue(response.contains("time --password='******' --arg=foo | log"));
+		assertTrue(response.contains("time --password='******' --arg=bar | log"));
 
 		assertTrue(response.contains("\\"totalElements\\":15"));
 
 	}
 
 	@Test
-	public void testSaveInvalidAppDefintions() throws Exception {
+	public void testSaveInvalidAppDefinitions() throws Exception {
 		mockMvc.perform(post("/streams/definitions/").param("name", "myStream").param("definition", "foo | bar")
 				.accept(MediaType.APPLICATION_JSON)).andDo(print()).andExpect(status().isBadRequest())
 				.andExpect(jsonPath("$[0].logref", is("InvalidStreamDefinitionException")))
@@ -369,7 +360,7 @@ public class StreamControllerTests {
 	}
 
 	@Test
-	public void testSaveInvalidAppDefintionsDueToParseException() throws Exception {
+	public void testSaveInvalidAppDefinitionsDueToParseException() throws Exception {
 		mockMvc.perform(post("/streams/definitions/").param("name", "myStream")
 				.param("definition", "foo --.spring.cloud.stream.metrics.properties=spring* | bar")
 				.accept(MediaType.APPLICATION_JSON)).andDo(print()).andExpect(status().isBadRequest())
@@ -619,7 +610,7 @@ public class StreamControllerTests {
 		when(appDeployer.status("myStream.log")).thenReturn(status);
 		mockMvc.perform(get("/streams/definitions/myStream").accept(MediaType.APPLICATION_JSON))
 				.andExpect(status().isOk()).andExpect(content().json("{name: \\"myStream\\"}"))
-				.andExpect(content().json("{dslText: \\"time --secret=****** | log\\"}"));
+				.andExpect(content().json("{dslText: \\"time --secret='******' | log\\"}"));
 	}
 
 	@Test
diff --git a/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/support/ArgumentSanitizerTest.java b/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/support/ArgumentSanitizerTest.java
index a42ccd86f..add550db1 100644
--- a/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/support/ArgumentSanitizerTest.java
+++ b/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/support/ArgumentSanitizerTest.java
@@ -20,6 +20,7 @@ import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
+import org.springframework.cloud.dataflow.core.StreamDefinition;
 import org.springframework.cloud.dataflow.server.controller.support.ArgumentSanitizer;
 
 /**
@@ -56,18 +57,23 @@ public class ArgumentSanitizerTest {
 
 	@Test
 	public void testHierarchicalPropertyNames() {
-		Assert.assertEquals("time --password=****** | log", ArgumentSanitizer.sanitizeStream("time --password=bar | log"));
+		Assert.assertEquals("time --password='******' | log",
+				sanitizer.sanitizeStream(new StreamDefinition("stream", "time --password=bar | log")));
 	}
 
 	@Test
 	public void testStreamMatcherWithHyphenDotChar() {
-		Assert.assertEquals("twitterstream --twitter.credentials.consumer-key=****** --twitter.credentials.consumer-secret=****** "
-				+ "--twitter.credentials.access-token=****** --twitter.credentials.access-token-secret=****** | filter --expression=#jsonPath(payload,'$.lang')=='en' | "
-				+ "twitter-sentiment --vocabulary=http://dl.bintray.com/test --model-fetch=output/test --model=http://dl.bintray.com/test | "
-				+ "field-value-counter --field-name=sentiment --name=sentiment", ArgumentSanitizer.sanitizeStream("twitterstream "
-				+ "--twitter.credentials.consumer-key=dadadfaf --twitter.credentials.consumer-secret=dadfdasfdads "
-				+ "--twitter.credentials.access-token=58849055-dfdae --twitter.credentials.access-token-secret=deteegdssa4466 | filter --expression=#jsonPath(payload,'$.lang')=='en' | "
-				+ "twitter-sentiment --vocabulary=http://dl.bintray.com/test --model-fetch=output/test --model=http://dl.bintray.com/test | "
-				+ "field-value-counter --field-name=sentiment --name=sentiment"));
+		Assert.assertEquals("twitterstream --twitter.credentials.access-token-secret='******' "
+						+ "--twitter.credentials.access-token='******' --twitter.credentials.consumer-secret='******' "
+						+ "--twitter.credentials.consumer-key='******' | "
+						+ "filter --expression=#jsonPath(payload,'$.lang')=='en' | "
+						+ "twitter-sentiment --vocabulary=http://dl.bintray.com/test --model-fetch=output/test "
+						+ "--model=http://dl.bintray.com/test | field-value-counter --field-name=sentiment --name=sentiment",
+				sanitizer.sanitizeStream(new StreamDefinition("stream", "twitterstream "
+						+ "--twitter.credentials.consumer-key=dadadfaf --twitter.credentials.consumer-secret=dadfdasfdads "
+						+ "--twitter.credentials.access-token=58849055-dfdae "
+						+ "--twitter.credentials.access-token-secret=deteegdssa4466 | filter --expression=#jsonPath(payload,'$.lang')=='en' | "
+						+ "twitter-sentiment --vocabulary=http://dl.bintray.com/test --model-fetch=output/test --model=http://dl.bintray.com/test | "
+						+ "field-value-counter --field-name=sentiment --name=sentiment")));
 	}
 }