Consolidates normalization of names and labels.

fixes gh-1561

spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocator.java

@@ -233,9 +233,8 @@ public class ConfigServicePropertySourceLocator implements PropertySourceLocator
 
 		Object[] args = new String[] { name, profile };
 		if (StringUtils.hasText(label)) {
-			if (label.contains("/")) {
-				label = label.replace("/", "(_)");
-			}
+			// workaround for Spring MVC matching / in paths
+			label = Environment.denormalize(label);
 			args = new String[] { name, profile, label };
 			path = path + "/{label}";
 		}

spring-cloud-config-client/src/main/java/org/springframework/cloud/config/environment/Environment.java

@@ -34,6 +34,11 @@ import com.fasterxml.jackson.annotation.JsonProperty;
  */
 public class Environment {
 
+	/**
+	 * "(_)" is uncommon in a git repo name, but "/" cannot be matched by Spring MVC.
+	 */
+	public static final String SLASH_PLACEHOLDER = "(_)";
+
 	private String name;
 
 	private String[] profiles = new String[0];
@@ -72,6 +77,32 @@ public class Environment {
 		this.state = state;
 	}
 
+	/**
+	 * Utility method for normalizing names and labels.
+	 * @param s String to normalize.
+	 * @return if s contains (_), replace with slash.
+	 */
+	public static String normalize(String s) {
+		if (s != null && s.contains(SLASH_PLACEHOLDER)) {
+			// "(_)" is uncommon in a git repo name, but "/" cannot be matched
+			// by Spring MVC
+			return s.replace(SLASH_PLACEHOLDER, "/");
+		}
+		return s;
+	}
+
+	/**
+	 * Utility method for denormalizing names and labels.
+	 * @param s String to denormalize.
+	 * @return if s contains slash, replace with (_).
+	 */
+	public static String denormalize(String s) {
+		if (s != null && s.contains("/")) {
+			return s.replace("/", SLASH_PLACEHOLDER);
+		}
+		return s;
+	}
+
 	public void add(PropertySource propertySource) {
 		this.propertySources.add(propertySource);
 	}

spring-cloud-config-client/src/test/java/org/springframework/cloud/config/environment/EnvironmentTests.java

@@ -0,0 +1,42 @@
+/*
+ * Copyright 2013-2020 the original author or authors.
+ *
+ * Licensed 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
+ *
+ *      https://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.springframework.cloud.config.environment;
+
+import org.junit.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class EnvironmentTests {
+
+	@Test
+	public void normalizeWorks() {
+		assertThat(Environment.normalize("abc123")).isEqualTo("abc123");
+		assertThat(Environment.normalize("abc(_)123")).isEqualTo("abc/123");
+		assertThat(Environment.normalize("abc(%5F)123")).isEqualTo("abc(%5F)123");
+		assertThat(Environment.normalize("abc%28_%29123")).isEqualTo("abc%28_%29123");
+		assertThat(Environment.normalize("abc%28%5F%29123")).isEqualTo("abc%28%5F%29123");
+	}
+
+	@Test
+	public void denormalizeWorks() {
+		assertThat(Environment.denormalize("abc123")).isEqualTo("abc123");
+		assertThat(Environment.denormalize("abc/123")).isEqualTo("abc(_)123");
+		assertThat(Environment.denormalize("abc%2F123")).isEqualTo("abc%2F123");
+		assertThat(Environment.denormalize("abc%25F123")).isEqualTo("abc%25F123");
+	}
+
+}

spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/EnvironmentController.java

@@ -131,16 +131,8 @@ public class EnvironmentController {
 
 	public Environment getEnvironment(String name, String profiles, String label,
 			boolean includeOrigin) {
-		if (name != null && name.contains("(_)")) {
-			// "(_)" is uncommon in a git repo name, but "/" cannot be matched
-			// by Spring MVC
-			name = name.replace("(_)", "/");
-		}
-		if (label != null && label.contains("(_)")) {
-			// "(_)" is uncommon in a git branch name, but "/" cannot be matched
-			// by Spring MVC
-			label = label.replace("(_)", "/");
-		}
+		name = Environment.normalize(name);
+		label = Environment.normalize(label);
 		Environment environment = this.repository.findOne(name, profiles, label,
 				includeOrigin);
 		if (!this.acceptEmpty

spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/resource/GenericResourceRepository.java

@@ -66,6 +66,9 @@ public class GenericResourceRepository
 			try {
 				for (int i = locations.length; i-- > 0;) {
 					String location = locations[i];
+					if (isInvalidEncodedLocation(location)) {
+						continue;
+					}
 					for (String local : getProfilePaths(profile, path)) {
 						if (!isInvalidPath(local) && !isInvalidEncodedPath(local)) {
 							Resource file = this.resourceLoader.getResource(location)
@@ -85,6 +88,41 @@ public class GenericResourceRepository
 		throw new NoSuchResourceException("Not found: " + path);
 	}
 
+	/**
+	 * Check whether the given location contains invalid escape sequences.
+	 * @param location the location to validate
+	 * @return {@code true} if the path is invalid, {@code false} otherwise
+	 */
+	private boolean isInvalidEncodedLocation(String location) {
+		if (location.contains("%")) {
+			try {
+				// Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8
+				// chars
+				String decodedPath = URLDecoder.decode(location, "UTF-8");
+				if (isInvalidLocation(decodedPath)) {
+					return true;
+				}
+				decodedPath = processPath(decodedPath);
+				if (isInvalidLocation(decodedPath)) {
+					return true;
+				}
+			}
+			catch (IllegalArgumentException | UnsupportedEncodingException ex) {
+				// Should never happen...
+			}
+		}
+		return isInvalidLocation(location);
+	}
+
+	private boolean isInvalidLocation(String location) {
+		boolean isInvalid = location.contains("..");
+
+		if (isInvalid && logger.isWarnEnabled()) {
+			logger.warn("Location contains \"..\"");
+		}
+		return isInvalid;
+	}
+
 	private Collection<String> getProfilePaths(String profiles, String path) {
 		Set<String> paths = new LinkedHashSet<>();
 		for (String profile : StringUtils.commaDelimitedListToSet(profiles)) {

spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/resource/ResourceController.java

@@ -135,8 +135,8 @@ public class ResourceController {
 
 	synchronized String retrieve(ServletWebRequest request, String name, String profile,
 			String label, String path, boolean resolvePlaceholders) throws IOException {
-		name = resolveName(name);
-		label = resolveLabel(label);
+		name = Environment.normalize(name);
+		label = Environment.normalize(label);
 		Resource resource = this.resourceRepository.findOne(name, profile, label, path);
 		if (checkNotModified(request, resource)) {
 			// Content was not modified. Just return.
@@ -199,8 +199,8 @@ public class ResourceController {
 
 	private synchronized byte[] binary(ServletWebRequest request, String name,
 			String profile, String label, String path) throws IOException {
-		name = resolveName(name);
-		label = resolveLabel(label);
+		name = Environment.normalize(name);
+		label = Environment.normalize(label);
 		Resource resource = this.resourceRepository.findOne(name, profile, label, path);
 		if (checkNotModified(request, resource)) {
 			// Content was not modified. Just return.
@@ -223,24 +223,6 @@ public class ResourceController {
 		return false;
 	}
 
-	private String resolveName(String name) {
-		if (name != null && name.contains("(_)")) {
-			// "(_)" is uncommon in a git repo name, but "/" cannot be matched
-			// by Spring MVC
-			name = name.replace("(_)", "/");
-		}
-		return name;
-	}
-
-	private String resolveLabel(String label) {
-		if (label != null && label.contains("(_)")) {
-			// "(_)" is uncommon in a git branch name, but "/" cannot be matched
-			// by Spring MVC
-			label = label.replace("(_)", "/");
-		}
-		return label;
-	}
-
 	@ExceptionHandler(NoSuchResourceException.class)
 	@ResponseStatus(HttpStatus.NOT_FOUND)
 	public void notFound(NoSuchResourceException e) {

spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/resource/GenericResourceRepositoryTests.java

@@ -100,9 +100,31 @@ public class GenericResourceRepositoryTests {
 		this.exception.expect(NoSuchResourceException.class);
 		this.nativeRepository
 				.setSearchLocations("file:./src/test/resources/test/{profile}");
-		this.repository.findOne("blah", "local", "master", "..%2F..%2Fdata-jdbc.sql");
 		this.output.expect(containsString(
 				"Path contains \"../\" after call to StringUtils#cleanPath"));
+		this.repository.findOne("blah", "local", "master", "..%2F..%2Fdata-jdbc.sql");
+	}
+
+	@Test
+	public void invalidPathWithPreviousDirectory() {
+		testInvalidPath("../");
+	}
+
+	@Test
+	public void invalidPathWithPreviousDirectoryEncodedSlash() {
+		testInvalidPath("..%2F");
+	}
+
+	@Test
+	public void invalidPathWithPreviousDirectoryAllEncoded() {
+		testInvalidPath("%2E%2E%2F");
+	}
+
+	private void testInvalidPath(String label) {
+		this.exception.expect(NoSuchResourceException.class);
+		this.nativeRepository.setSearchLocations("file:./src/test/resources/test/local");
+		this.output.expect(containsString("Location contains \"..\""));
+		this.repository.findOne("blah", "local", label, "foo.properties");
 	}
 
 }