From cef4478b7b917033a8a01a6e633f6f59aab67e59 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 30 Oct 2019 15:59:44 +0100 Subject: [PATCH] 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 --- .../mock/web/MockServletContext.java | 51 ++- .../mock/web/MockServletContextTests.java | 346 ++++++++++-------- .../mock/web/test/MockServletContext.java | 51 ++- 3 files changed, 265 insertions(+), 183 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/mock/web/MockServletContext.java b/spring-test/src/main/java/org/springframework/mock/web/MockServletContext.java index 97fed0c5d1..7dab1c8c21 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/MockServletContext.java +++ b/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 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; } diff --git a/spring-test/src/test/java/org/springframework/mock/web/MockServletContextTests.java b/spring-test/src/test/java/org/springframework/mock/web/MockServletContextTests.java index 48c8e32ee3..fd26ff1e25 100644 --- a/spring-test/src/test/java/org/springframework/mock/web/MockServletContextTests.java +++ b/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"); - - - @Test - void listFiles() { - Set paths = sc.getResourcePaths("/web"); - assertThat(paths).isNotNull(); - assertThat(paths.contains("/web/MockServletContextTests.class")).isTrue(); - } - - @Test - void listSubdirectories() { - Set paths = sc.getResourcePaths("/"); - assertThat(paths).isNotNull(); - assertThat(paths.contains("/web/")).isTrue(); - } - - @Test - void listNonDirectory() { - Set paths = sc.getResourcePaths("/web/MockServletContextTests.class"); - assertThat(paths).isNull(); - } - - @Test - void listInvalidPath() { - Set paths = sc.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 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: - * Testing Spring managed servlet - */ - @Test - void getMimeTypeWithCustomConfiguredType() { - sc.addMimeType("enigma", new MediaType("text", "enigma")); - assertThat(sc.getMimeType("filename.enigma")).isEqualTo("text/enigma"); - } - - @Test - void servletVersion() { - assertThat(sc.getMajorVersion()).isEqualTo(3); - assertThat(sc.getMinorVersion()).isEqualTo(1); - assertThat(sc.getEffectiveMajorVersion()).isEqualTo(3); - assertThat(sc.getEffectiveMinorVersion()).isEqualTo(1); - - 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 registerAndUnregisterNamedDispatcher() throws Exception { - final String name = "test-servlet"; - final String url = "/test"; - assertThat(sc.getNamedDispatcher(name)).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); - - sc.unregisterNamedDispatcher(name); - assertThat(sc.getNamedDispatcher(name)).isNull(); - } - - @Test - void getNamedDispatcherForDefaultServlet() throws Exception { - final String name = "default"; - RequestDispatcher namedDispatcher = sc.getNamedDispatcher(name); - assertThat(namedDispatcher).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(); - } + @Nested + @DisplayName("with DefaultResourceLoader") + class MockServletContextWithDefaultResourceLoaderTests { + + private final MockServletContext servletContext = new MockServletContext("org/springframework/mock"); + + @Test + void getResourcePaths() { + Set paths = servletContext.getResourcePaths("/web"); + assertThat(paths).isNotNull(); + assertThat(paths.contains("/web/MockServletContextTests.class")).isTrue(); + } + + @Test + void getResourcePathsWithSubdirectories() { + Set paths = servletContext.getResourcePaths("/"); + assertThat(paths).isNotNull(); + assertThat(paths.contains("/web/")).isTrue(); + } + + @Test + void getResourcePathsWithNonDirectory() { + Set paths = servletContext.getResourcePaths("/web/MockServletContextTests.class"); + assertThat(paths).isNull(); + } + + @Test + void getResourcePathsWithInvalidPath() { + Set paths = servletContext.getResourcePaths("/web/invalid"); + assertThat(paths).isNull(); + } + + @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: + * Testing Spring managed servlet + */ + @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 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 filterRegistrations = servletContext.getFilterRegistrations(); + assertThat(filterRegistrations).isNotNull(); + assertThat(filterRegistrations.size()).isEqualTo(0); + } - /** - * @since 4.1.2 - */ - @Test - void getServletRegistrations() { - Map servletRegistrations = sc.getServletRegistrations(); - assertThat(servletRegistrations).isNotNull(); - assertThat(servletRegistrations.size()).isEqualTo(0); } /** - * @since 4.1.2 + * @since 5.1.11 */ - @Test - void getFilterRegistration() { - assertThat(sc.getFilterRegistration("filter")).isNull(); - } + @Nested + @DisplayName("with FileSystemResourceLoader") + class MockServletContextWithFileSystemResourceLoaderTests { + + private final MockServletContext servletContext = + new MockServletContext( "org/springframework/mock", new FileSystemResourceLoader()); + + @Test + void getResourcePathsWithRelativePathToWindowsCDrive() { + Set paths = servletContext.getResourcePaths("C:\\temp"); + assertThat(paths).isNull(); + } + + @Test + void getResourceWithRelativePathToWindowsCDrive() throws Exception { + URL resource = servletContext.getResource("C:\\temp"); + assertThat(resource).isNull(); + } + + @Test + void getResourceAsStreamWithRelativePathToWindowsCDrive() { + InputStream inputStream = servletContext.getResourceAsStream("C:\\temp"); + assertThat(inputStream).isNull(); + } + + @Test + void getRealPathWithRelativePathToWindowsCDrive() { + String realPath = servletContext.getRealPath("C:\\temp"); + + if (OS.WINDOWS.isCurrentOs()) { + assertThat(realPath).isNull(); + } + else { + assertThat(realPath).isNotNull(); + } + } - /** - * @since 4.1.2 - */ - @Test - void getFilterRegistrations() { - Map filterRegistrations = sc.getFilterRegistrations(); - assertThat(filterRegistrations).isNotNull(); - assertThat(filterRegistrations.size()).isEqualTo(0); } } diff --git a/spring-web/src/test/java/org/springframework/mock/web/test/MockServletContext.java b/spring-web/src/test/java/org/springframework/mock/web/test/MockServletContext.java index 3b78ed50ee..32fa355b86 100644 --- a/spring-web/src/test/java/org/springframework/mock/web/test/MockServletContext.java +++ b/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 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; } -- GitLab