Treat InvalidPathException like an IOException in MockServletContext

Prior to this commit, if MockServletContext was configured with a
FileSystemResourceLoader, invocations of the following methods on a
Microsoft Windows operating system resulted in an InvalidPathException
if the supplied path contained a colon (such as "C:\\temp"). This is
inconsistent with the behavior on non-Windows operating systems. In
addition, for comparable errors resulting in an IOException, those
methods (except getRealPath()) return null instead of throwing the
exception.

- getResourcePaths()
- getResource()
- getResourceAsStream()
- getRealPath()

This commit makes handling of InvalidPathException and IOException
consistent for these methods: both exceptions now result in null be
returned by these methods.

Closes gh-23717

spring-test/src/main/java/org/springframework/mock/web/MockServletContext.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -21,6 +21,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.nio.file.InvalidPathException;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.EventListener;
@@ -294,8 +295,10 @@ public class MockServletContext implements ServletContext {
 	@Nullable
 	public Set<String> getResourcePaths(String path) {
 		String actualPath = (path.endsWith("/") ? path : path + "/");
-		Resource resource = this.resourceLoader.getResource(getResourceLocation(actualPath));
+		String resourceLocation = getResourceLocation(actualPath);
+		Resource resource = null;
 		try {
+			resource = this.resourceLoader.getResource(resourceLocation);
 			File file = resource.getFile();
 			String[] fileList = file.list();
 			if (ObjectUtils.isEmpty(fileList)) {
@@ -311,9 +314,10 @@ public class MockServletContext implements ServletContext {
 			}
 			return resourcePaths;
 		}
-		catch (IOException ex) {
+		catch (InvalidPathException | IOException ex ) {
 			if (logger.isWarnEnabled()) {
-				logger.warn("Could not get resource paths for " + resource, ex);
+				logger.warn("Could not get resource paths for " +
+						(resource != null ? resource : resourceLocation), ex);
 			}
 			return null;
 		}
@@ -322,19 +326,22 @@ public class MockServletContext implements ServletContext {
 	@Override
 	@Nullable
 	public URL getResource(String path) throws MalformedURLException {
-		Resource resource = this.resourceLoader.getResource(getResourceLocation(path));
-		if (!resource.exists()) {
-			return null;
-		}
+		String resourceLocation = getResourceLocation(path);
+		Resource resource = null;
 		try {
+			resource = this.resourceLoader.getResource(resourceLocation);
+			if (!resource.exists()) {
+				return null;
+			}
 			return resource.getURL();
 		}
 		catch (MalformedURLException ex) {
 			throw ex;
 		}
-		catch (IOException ex) {
+		catch (InvalidPathException | IOException ex) {
 			if (logger.isWarnEnabled()) {
-				logger.warn("Could not get URL for " + resource, ex);
+				logger.warn("Could not get URL for resource " +
+						(resource != null ? resource : resourceLocation), ex);
 			}
 			return null;
 		}
@@ -343,16 +350,19 @@ public class MockServletContext implements ServletContext {
 	@Override
 	@Nullable
 	public InputStream getResourceAsStream(String path) {
-		Resource resource = this.resourceLoader.getResource(getResourceLocation(path));
-		if (!resource.exists()) {
-			return null;
-		}
+		String resourceLocation = getResourceLocation(path);
+		Resource resource = null;
 		try {
+			resource = this.resourceLoader.getResource(resourceLocation);
+			if (!resource.exists()) {
+				return null;
+			}
 			return resource.getInputStream();
 		}
-		catch (IOException ex) {
+		catch (InvalidPathException | IOException ex) {
 			if (logger.isWarnEnabled()) {
-				logger.warn("Could not open InputStream for " + resource, ex);
+				logger.warn("Could not open InputStream for resource " +
+						(resource != null ? resource : resourceLocation), ex);
 			}
 			return null;
 		}
@@ -459,13 +469,16 @@ public class MockServletContext implements ServletContext {
 	@Override
 	@Nullable
 	public String getRealPath(String path) {
-		Resource resource = this.resourceLoader.getResource(getResourceLocation(path));
+		String resourceLocation = getResourceLocation(path);
+		Resource resource = null;
 		try {
+			resource = this.resourceLoader.getResource(resourceLocation);
 			return resource.getFile().getAbsolutePath();
 		}
-		catch (IOException ex) {
+		catch (InvalidPathException | IOException ex) {
 			if (logger.isWarnEnabled()) {
-				logger.warn("Could not determine real path of resource " + resource, ex);
+				logger.warn("Could not determine real path of resource " +
+						(resource != null ? resource : resourceLocation), ex);
 			}
 			return null;
 		}

spring-test/src/test/java/org/springframework/mock/web/MockServletContextTests.java

@@ -16,6 +16,8 @@
 
 package org.springframework.mock.web;
 
+import java.io.InputStream;
+import java.net.URL;
 import java.util.Map;
 import java.util.Set;
 
@@ -23,171 +25,225 @@ import javax.servlet.FilterRegistration;
 import javax.servlet.RequestDispatcher;
 import javax.servlet.ServletRegistration;
 
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.OS;
 
+import org.springframework.core.io.FileSystemResourceLoader;
 import org.springframework.http.MediaType;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
 /**
+ * Unit tests for {@link MockServletContext}.
+ *
  * @author Juergen Hoeller
  * @author Chris Beams
  * @author Sam Brannen
  * @since 19.02.2006
  */
+@DisplayName("MockServletContext unit tests")
 class MockServletContextTests {
 
-	private final MockServletContext sc = new MockServletContext("org/springframework/mock");
+	@Nested
+	@DisplayName("with DefaultResourceLoader")
+	class MockServletContextWithDefaultResourceLoaderTests {
 
+		private final MockServletContext servletContext = new MockServletContext("org/springframework/mock");
 
-	@Test
-	void listFiles() {
-		Set<String> paths = sc.getResourcePaths("/web");
-		assertThat(paths).isNotNull();
-		assertThat(paths.contains("/web/MockServletContextTests.class")).isTrue();
-	}
+		@Test
+		void getResourcePaths() {
+			Set<String> paths = servletContext.getResourcePaths("/web");
+			assertThat(paths).isNotNull();
+			assertThat(paths.contains("/web/MockServletContextTests.class")).isTrue();
+		}
 
-	@Test
-	void listSubdirectories() {
-		Set<String> paths = sc.getResourcePaths("/");
-		assertThat(paths).isNotNull();
-		assertThat(paths.contains("/web/")).isTrue();
-	}
+		@Test
+		void getResourcePathsWithSubdirectories() {
+			Set<String> paths = servletContext.getResourcePaths("/");
+			assertThat(paths).isNotNull();
+			assertThat(paths.contains("/web/")).isTrue();
+		}
 
-	@Test
-	void listNonDirectory() {
-		Set<String> paths = sc.getResourcePaths("/web/MockServletContextTests.class");
-		assertThat(paths).isNull();
-	}
+		@Test
+		void getResourcePathsWithNonDirectory() {
+			Set<String> paths = servletContext.getResourcePaths("/web/MockServletContextTests.class");
+			assertThat(paths).isNull();
+		}
 
-	@Test
-	void listInvalidPath() {
-		Set<String> paths = sc.getResourcePaths("/web/invalid");
-		assertThat(paths).isNull();
-	}
+		@Test
+		void getResourcePathsWithInvalidPath() {
+			Set<String> paths = servletContext.getResourcePaths("/web/invalid");
+			assertThat(paths).isNull();
+		}
 
-	@Test
-	void registerContextAndGetContext() {
-		MockServletContext sc2 = new MockServletContext();
-		sc.setContextPath("/");
-		sc.registerContext("/second", sc2);
-		assertThat(sc.getContext("/")).isSameAs(sc);
-		assertThat(sc.getContext("/second")).isSameAs(sc2);
-	}
+		@Test
+		void registerContextAndGetContext() {
+			MockServletContext sc2 = new MockServletContext();
+			servletContext.setContextPath("/");
+			servletContext.registerContext("/second", sc2);
+			assertThat(servletContext.getContext("/")).isSameAs(servletContext);
+			assertThat(servletContext.getContext("/second")).isSameAs(sc2);
+		}
+
+		@Test
+		void getMimeType() {
+			assertThat(servletContext.getMimeType("test.html")).isEqualTo("text/html");
+			assertThat(servletContext.getMimeType("test.gif")).isEqualTo("image/gif");
+			assertThat(servletContext.getMimeType("test.foobar")).isNull();
+		}
+
+		/**
+		 * Introduced to dispel claims in a thread on Stack Overflow:
+		 * <a href="https://stackoverflow.com/questions/22986109/testing-spring-managed-servlet">Testing Spring managed servlet</a>
+		 */
+		@Test
+		void getMimeTypeWithCustomConfiguredType() {
+			servletContext.addMimeType("enigma", new MediaType("text", "enigma"));
+			assertThat(servletContext.getMimeType("filename.enigma")).isEqualTo("text/enigma");
+		}
+
+		@Test
+		void servletVersion() {
+			assertThat(servletContext.getMajorVersion()).isEqualTo(3);
+			assertThat(servletContext.getMinorVersion()).isEqualTo(1);
+			assertThat(servletContext.getEffectiveMajorVersion()).isEqualTo(3);
+			assertThat(servletContext.getEffectiveMinorVersion()).isEqualTo(1);
+
+			servletContext.setMajorVersion(4);
+			servletContext.setMinorVersion(0);
+			servletContext.setEffectiveMajorVersion(4);
+			servletContext.setEffectiveMinorVersion(0);
+			assertThat(servletContext.getMajorVersion()).isEqualTo(4);
+			assertThat(servletContext.getMinorVersion()).isEqualTo(0);
+			assertThat(servletContext.getEffectiveMajorVersion()).isEqualTo(4);
+			assertThat(servletContext.getEffectiveMinorVersion()).isEqualTo(0);
+		}
+
+		@Test
+		void registerAndUnregisterNamedDispatcher() throws Exception {
+			final String name = "test-servlet";
+			final String url = "/test";
+			assertThat(servletContext.getNamedDispatcher(name)).isNull();
+
+			servletContext.registerNamedDispatcher(name, new MockRequestDispatcher(url));
+			RequestDispatcher namedDispatcher = servletContext.getNamedDispatcher(name);
+			assertThat(namedDispatcher).isNotNull();
+			MockHttpServletResponse response = new MockHttpServletResponse();
+			namedDispatcher.forward(new MockHttpServletRequest(servletContext), response);
+			assertThat(response.getForwardedUrl()).isEqualTo(url);
+
+			servletContext.unregisterNamedDispatcher(name);
+			assertThat(servletContext.getNamedDispatcher(name)).isNull();
+		}
+
+		@Test
+		void getNamedDispatcherForDefaultServlet() throws Exception {
+			final String name = "default";
+			RequestDispatcher namedDispatcher = servletContext.getNamedDispatcher(name);
+			assertThat(namedDispatcher).isNotNull();
+
+			MockHttpServletResponse response = new MockHttpServletResponse();
+			namedDispatcher.forward(new MockHttpServletRequest(servletContext), response);
+			assertThat(response.getForwardedUrl()).isEqualTo(name);
+		}
+
+		@Test
+		void setDefaultServletName() throws Exception {
+			final String originalDefault = "default";
+			final String newDefault = "test";
+			assertThat(servletContext.getNamedDispatcher(originalDefault)).isNotNull();
+
+			servletContext.setDefaultServletName(newDefault);
+			assertThat(servletContext.getDefaultServletName()).isEqualTo(newDefault);
+			assertThat(servletContext.getNamedDispatcher(originalDefault)).isNull();
+
+			RequestDispatcher namedDispatcher = servletContext.getNamedDispatcher(newDefault);
+			assertThat(namedDispatcher).isNotNull();
+			MockHttpServletResponse response = new MockHttpServletResponse();
+			namedDispatcher.forward(new MockHttpServletRequest(servletContext), response);
+			assertThat(response.getForwardedUrl()).isEqualTo(newDefault);
+		}
+
+		/**
+		 * @since 4.1.2
+		 */
+		@Test
+		void getServletRegistration() {
+			assertThat(servletContext.getServletRegistration("servlet")).isNull();
+		}
+
+		/**
+		 * @since 4.1.2
+		 */
+		@Test
+		void getServletRegistrations() {
+			Map<String, ? extends ServletRegistration> servletRegistrations = servletContext.getServletRegistrations();
+			assertThat(servletRegistrations).isNotNull();
+			assertThat(servletRegistrations.size()).isEqualTo(0);
+		}
+
+		/**
+		 * @since 4.1.2
+		 */
+		@Test
+		void getFilterRegistration() {
+			assertThat(servletContext.getFilterRegistration("filter")).isNull();
+		}
+
+		/**
+		 * @since 4.1.2
+		 */
+		@Test
+		void getFilterRegistrations() {
+			Map<String, ? extends FilterRegistration> filterRegistrations = servletContext.getFilterRegistrations();
+			assertThat(filterRegistrations).isNotNull();
+			assertThat(filterRegistrations.size()).isEqualTo(0);
+		}
 
-	@Test
-	void getMimeType() {
-		assertThat(sc.getMimeType("test.html")).isEqualTo("text/html");
-		assertThat(sc.getMimeType("test.gif")).isEqualTo("image/gif");
-		assertThat(sc.getMimeType("test.foobar")).isNull();
 	}
 
 	/**
-	 * Introduced to dispel claims in a thread on Stack Overflow:
-	 * <a href="https://stackoverflow.com/questions/22986109/testing-spring-managed-servlet">Testing Spring managed servlet</a>
+	 * @since 5.1.11
 	 */
-	@Test
-	void getMimeTypeWithCustomConfiguredType() {
-		sc.addMimeType("enigma", new MediaType("text", "enigma"));
-		assertThat(sc.getMimeType("filename.enigma")).isEqualTo("text/enigma");
-	}
+	@Nested
+	@DisplayName("with FileSystemResourceLoader")
+	class MockServletContextWithFileSystemResourceLoaderTests {
 
-	@Test
-	void servletVersion() {
-		assertThat(sc.getMajorVersion()).isEqualTo(3);
-		assertThat(sc.getMinorVersion()).isEqualTo(1);
-		assertThat(sc.getEffectiveMajorVersion()).isEqualTo(3);
-		assertThat(sc.getEffectiveMinorVersion()).isEqualTo(1);
+		private final MockServletContext servletContext =
+				new MockServletContext( "org/springframework/mock", new FileSystemResourceLoader());
 
-		sc.setMajorVersion(4);
-		sc.setMinorVersion(0);
-		sc.setEffectiveMajorVersion(4);
-		sc.setEffectiveMinorVersion(0);
-		assertThat(sc.getMajorVersion()).isEqualTo(4);
-		assertThat(sc.getMinorVersion()).isEqualTo(0);
-		assertThat(sc.getEffectiveMajorVersion()).isEqualTo(4);
-		assertThat(sc.getEffectiveMinorVersion()).isEqualTo(0);
-	}
+		@Test
+		void getResourcePathsWithRelativePathToWindowsCDrive() {
+			Set<String> paths = servletContext.getResourcePaths("C:\\temp");
+			assertThat(paths).isNull();
+		}
 
-	@Test
-	void registerAndUnregisterNamedDispatcher() throws Exception {
-		final String name = "test-servlet";
-		final String url = "/test";
-		assertThat(sc.getNamedDispatcher(name)).isNull();
+		@Test
+		void getResourceWithRelativePathToWindowsCDrive() throws Exception {
+			URL resource = servletContext.getResource("C:\\temp");
+			assertThat(resource).isNull();
+		}
 
-		sc.registerNamedDispatcher(name, new MockRequestDispatcher(url));
-		RequestDispatcher namedDispatcher = sc.getNamedDispatcher(name);
-		assertThat(namedDispatcher).isNotNull();
-		MockHttpServletResponse response = new MockHttpServletResponse();
-		namedDispatcher.forward(new MockHttpServletRequest(sc), response);
-		assertThat(response.getForwardedUrl()).isEqualTo(url);
+		@Test
+		void getResourceAsStreamWithRelativePathToWindowsCDrive() {
+			InputStream inputStream = servletContext.getResourceAsStream("C:\\temp");
+			assertThat(inputStream).isNull();
+		}
 
-		sc.unregisterNamedDispatcher(name);
-		assertThat(sc.getNamedDispatcher(name)).isNull();
-	}
+		@Test
+		void getRealPathWithRelativePathToWindowsCDrive() {
+			String realPath = servletContext.getRealPath("C:\\temp");
 
-	@Test
-	void getNamedDispatcherForDefaultServlet() throws Exception {
-		final String name = "default";
-		RequestDispatcher namedDispatcher = sc.getNamedDispatcher(name);
-		assertThat(namedDispatcher).isNotNull();
+			if (OS.WINDOWS.isCurrentOs()) {
+				assertThat(realPath).isNull();
+			}
+			else {
+				assertThat(realPath).isNotNull();
+			}
+		}
 
-		MockHttpServletResponse response = new MockHttpServletResponse();
-		namedDispatcher.forward(new MockHttpServletRequest(sc), response);
-		assertThat(response.getForwardedUrl()).isEqualTo(name);
-	}
-
-	@Test
-	void setDefaultServletName() throws Exception {
-		final String originalDefault = "default";
-		final String newDefault = "test";
-		assertThat(sc.getNamedDispatcher(originalDefault)).isNotNull();
-
-		sc.setDefaultServletName(newDefault);
-		assertThat(sc.getDefaultServletName()).isEqualTo(newDefault);
-		assertThat(sc.getNamedDispatcher(originalDefault)).isNull();
-
-		RequestDispatcher namedDispatcher = sc.getNamedDispatcher(newDefault);
-		assertThat(namedDispatcher).isNotNull();
-		MockHttpServletResponse response = new MockHttpServletResponse();
-		namedDispatcher.forward(new MockHttpServletRequest(sc), response);
-		assertThat(response.getForwardedUrl()).isEqualTo(newDefault);
-	}
-
-	/**
-	 * @since 4.1.2
-	 */
-	@Test
-	void getServletRegistration() {
-		assertThat(sc.getServletRegistration("servlet")).isNull();
-	}
-
-	/**
-	 * @since 4.1.2
-	 */
-	@Test
-	void getServletRegistrations() {
-		Map<String, ? extends ServletRegistration> servletRegistrations = sc.getServletRegistrations();
-		assertThat(servletRegistrations).isNotNull();
-		assertThat(servletRegistrations.size()).isEqualTo(0);
-	}
-
-	/**
-	 * @since 4.1.2
-	 */
-	@Test
-	void getFilterRegistration() {
-		assertThat(sc.getFilterRegistration("filter")).isNull();
-	}
-
-	/**
-	 * @since 4.1.2
-	 */
-	@Test
-	void getFilterRegistrations() {
-		Map<String, ? extends FilterRegistration> filterRegistrations = sc.getFilterRegistrations();
-		assertThat(filterRegistrations).isNotNull();
-		assertThat(filterRegistrations.size()).isEqualTo(0);
 	}
 
 }

spring-web/src/test/java/org/springframework/mock/web/test/MockServletContext.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -21,6 +21,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.nio.file.InvalidPathException;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.EventListener;
@@ -294,8 +295,10 @@ public class MockServletContext implements ServletContext {
 	@Nullable
 	public Set<String> getResourcePaths(String path) {
 		String actualPath = (path.endsWith("/") ? path : path + "/");
-		Resource resource = this.resourceLoader.getResource(getResourceLocation(actualPath));
+		String resourceLocation = getResourceLocation(actualPath);
+		Resource resource = null;
 		try {
+			resource = this.resourceLoader.getResource(resourceLocation);
 			File file = resource.getFile();
 			String[] fileList = file.list();
 			if (ObjectUtils.isEmpty(fileList)) {
@@ -311,9 +314,10 @@ public class MockServletContext implements ServletContext {
 			}
 			return resourcePaths;
 		}
-		catch (IOException ex) {
+		catch (InvalidPathException | IOException ex ) {
 			if (logger.isWarnEnabled()) {
-				logger.warn("Could not get resource paths for " + resource, ex);
+				logger.warn("Could not get resource paths for " +
+						(resource != null ? resource : resourceLocation), ex);
 			}
 			return null;
 		}
@@ -322,19 +326,22 @@ public class MockServletContext implements ServletContext {
 	@Override
 	@Nullable
 	public URL getResource(String path) throws MalformedURLException {
-		Resource resource = this.resourceLoader.getResource(getResourceLocation(path));
-		if (!resource.exists()) {
-			return null;
-		}
+		String resourceLocation = getResourceLocation(path);
+		Resource resource = null;
 		try {
+			resource = this.resourceLoader.getResource(resourceLocation);
+			if (!resource.exists()) {
+				return null;
+			}
 			return resource.getURL();
 		}
 		catch (MalformedURLException ex) {
 			throw ex;
 		}
-		catch (IOException ex) {
+		catch (InvalidPathException | IOException ex) {
 			if (logger.isWarnEnabled()) {
-				logger.warn("Could not get URL for " + resource, ex);
+				logger.warn("Could not get URL for resource " +
+						(resource != null ? resource : resourceLocation), ex);
 			}
 			return null;
 		}
@@ -343,16 +350,19 @@ public class MockServletContext implements ServletContext {
 	@Override
 	@Nullable
 	public InputStream getResourceAsStream(String path) {
-		Resource resource = this.resourceLoader.getResource(getResourceLocation(path));
-		if (!resource.exists()) {
-			return null;
-		}
+		String resourceLocation = getResourceLocation(path);
+		Resource resource = null;
 		try {
+			resource = this.resourceLoader.getResource(resourceLocation);
+			if (!resource.exists()) {
+				return null;
+			}
 			return resource.getInputStream();
 		}
-		catch (IOException ex) {
+		catch (InvalidPathException | IOException ex) {
 			if (logger.isWarnEnabled()) {
-				logger.warn("Could not open InputStream for " + resource, ex);
+				logger.warn("Could not open InputStream for resource " +
+						(resource != null ? resource : resourceLocation), ex);
 			}
 			return null;
 		}
@@ -459,13 +469,16 @@ public class MockServletContext implements ServletContext {
 	@Override
 	@Nullable
 	public String getRealPath(String path) {
-		Resource resource = this.resourceLoader.getResource(getResourceLocation(path));
+		String resourceLocation = getResourceLocation(path);
+		Resource resource = null;
 		try {
+			resource = this.resourceLoader.getResource(resourceLocation);
 			return resource.getFile().getAbsolutePath();
 		}
-		catch (IOException ex) {
+		catch (InvalidPathException | IOException ex) {
 			if (logger.isWarnEnabled()) {
-				logger.warn("Could not determine real path of resource " + resource, ex);
+				logger.warn("Could not determine real path of resource " +
+						(resource != null ? resource : resourceLocation), ex);
 			}
 			return null;
 		}