From b71e686cbd0689a3dbca55cc345de3e212ef7391 Mon Sep 17 00:00:00 2001 From: chenrl Date: Sun, 21 Mar 2021 16:46:15 +0800 Subject: [PATCH 1/2] Fix ServletContextResource isFile check Prior to this commit, `ServletContextResource` could rely on `ServletContext#getRealPath` to check whether a resource exists. This behavior is not enforced on some Servlet containers, as this method is only meant to translate virtual paths to real paths, but not necessarily check for the existence of the file. See https://bz.apache.org/bugzilla/show_bug.cgi?id=55837#c3 for a rationale of this behavior in Tomcat. This commit enforces an additional check, resolving the path as a `File` and checking that is exists and is a file. Closes gh-26707 --- .../web/context/support/ServletContextResource.java | 9 +++++++-- .../web/context/support/ResourceTests.java | 10 ++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/context/support/ServletContextResource.java b/spring-web/src/main/java/org/springframework/web/context/support/ServletContextResource.java index 9f19b76d99..ad236509d8 100644 --- a/spring-web/src/main/java/org/springframework/web/context/support/ServletContextResource.java +++ b/spring-web/src/main/java/org/springframework/web/context/support/ServletContextResource.java @@ -139,10 +139,15 @@ public class ServletContextResource extends AbstractFileResolvingResource implem return true; } else { - return (this.servletContext.getRealPath(this.path) != null); + String realPath = this.servletContext.getRealPath(this.path); + if (realPath == null) { + return false; + } + File file = new File(realPath); + return (file.exists() && file.isFile()); } } - catch (MalformedURLException ex) { + catch (IOException ex) { return false; } } diff --git a/spring-web/src/test/java/org/springframework/web/context/support/ResourceTests.java b/spring-web/src/test/java/org/springframework/web/context/support/ResourceTests.java index 3408388718..937885e107 100644 --- a/spring-web/src/test/java/org/springframework/web/context/support/ResourceTests.java +++ b/spring-web/src/test/java/org/springframework/web/context/support/ResourceTests.java @@ -36,6 +36,8 @@ public class ResourceTests { MockServletContext sc = new MockServletContext(); Resource resource = new ServletContextResource(sc, "org/springframework/core/io/Resource.class"); doTestResource(resource); + Resource resourceNotExists = new ServletContextResource(sc, "org/springframework/core/io/ResourceNotExists.class"); + doTestNotExistsResource(resourceNotExists); assertThat(new ServletContextResource(sc, "org/springframework/core/../core/io/./Resource.class")).isEqualTo(resource); } @@ -48,6 +50,9 @@ public class ResourceTests { } private void doTestResource(Resource resource) throws IOException { + assertThat(resource.getFile()).isNotNull(); + assertThat(resource.exists()).isTrue(); + assertThat(resource.isFile()).isTrue(); assertThat(resource.getFilename()).isEqualTo("Resource.class"); assertThat(resource.getURL().getFile().endsWith("Resource.class")).isTrue(); @@ -61,4 +66,9 @@ public class ResourceTests { assertThat(relative2.getURL().getFile().endsWith("ResourcePatternResolver.class")).isTrue(); assertThat(relative2.exists()).isTrue(); } + + private void doTestNotExistsResource(Resource resource) throws IOException { + assertThat(resource.exists()).isFalse(); + assertThat(resource.isFile()).isFalse(); + } } From 9465110917d5dd90db39b6f1f1f51b1e7f2fe539 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Mon, 26 Sep 2022 21:47:41 +0200 Subject: [PATCH 2/2] Polish See gh-26707 --- .../support/ServletContextResource.java | 2 +- .../web/context/support/ResourceTests.java | 74 --------------- .../support/ServletContextResourceTests.java | 91 +++++++++++++++++++ .../web/context/support/folder/other.txt | 0 .../web/context/support/relative.txt | 0 .../web/context/support/resource.txt | 0 6 files changed, 92 insertions(+), 75 deletions(-) delete mode 100644 spring-web/src/test/java/org/springframework/web/context/support/ResourceTests.java create mode 100644 spring-web/src/test/java/org/springframework/web/context/support/ServletContextResourceTests.java create mode 100644 spring-web/src/test/resources/org/springframework/web/context/support/folder/other.txt create mode 100644 spring-web/src/test/resources/org/springframework/web/context/support/relative.txt create mode 100644 spring-web/src/test/resources/org/springframework/web/context/support/resource.txt diff --git a/spring-web/src/main/java/org/springframework/web/context/support/ServletContextResource.java b/spring-web/src/main/java/org/springframework/web/context/support/ServletContextResource.java index ad236509d8..fff20515e3 100644 --- a/spring-web/src/main/java/org/springframework/web/context/support/ServletContextResource.java +++ b/spring-web/src/main/java/org/springframework/web/context/support/ServletContextResource.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2022 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. diff --git a/spring-web/src/test/java/org/springframework/web/context/support/ResourceTests.java b/spring-web/src/test/java/org/springframework/web/context/support/ResourceTests.java deleted file mode 100644 index 937885e107..0000000000 --- a/spring-web/src/test/java/org/springframework/web/context/support/ResourceTests.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * 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. - * 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.web.context.support; - -import java.io.IOException; - -import org.junit.jupiter.api.Test; - -import org.springframework.core.io.Resource; -import org.springframework.web.testfixture.servlet.MockServletContext; - -import static org.assertj.core.api.Assertions.assertThat; - -/** - * @author Chris Beams - * @see org.springframework.core.io.ResourceTests - */ -public class ResourceTests { - - @Test - public void testServletContextResource() throws IOException { - MockServletContext sc = new MockServletContext(); - Resource resource = new ServletContextResource(sc, "org/springframework/core/io/Resource.class"); - doTestResource(resource); - Resource resourceNotExists = new ServletContextResource(sc, "org/springframework/core/io/ResourceNotExists.class"); - doTestNotExistsResource(resourceNotExists); - assertThat(new ServletContextResource(sc, "org/springframework/core/../core/io/./Resource.class")).isEqualTo(resource); - } - - @Test - public void testServletContextResourceWithRelativePath() throws IOException { - MockServletContext sc = new MockServletContext(); - Resource resource = new ServletContextResource(sc, "dir/"); - Resource relative = resource.createRelative("subdir"); - assertThat(relative).isEqualTo(new ServletContextResource(sc, "dir/subdir")); - } - - private void doTestResource(Resource resource) throws IOException { - assertThat(resource.getFile()).isNotNull(); - assertThat(resource.exists()).isTrue(); - assertThat(resource.isFile()).isTrue(); - assertThat(resource.getFilename()).isEqualTo("Resource.class"); - assertThat(resource.getURL().getFile().endsWith("Resource.class")).isTrue(); - - Resource relative1 = resource.createRelative("ClassPathResource.class"); - assertThat(relative1.getFilename()).isEqualTo("ClassPathResource.class"); - assertThat(relative1.getURL().getFile().endsWith("ClassPathResource.class")).isTrue(); - assertThat(relative1.exists()).isTrue(); - - Resource relative2 = resource.createRelative("support/ResourcePatternResolver.class"); - assertThat(relative2.getFilename()).isEqualTo("ResourcePatternResolver.class"); - assertThat(relative2.getURL().getFile().endsWith("ResourcePatternResolver.class")).isTrue(); - assertThat(relative2.exists()).isTrue(); - } - - private void doTestNotExistsResource(Resource resource) throws IOException { - assertThat(resource.exists()).isFalse(); - assertThat(resource.isFile()).isFalse(); - } -} diff --git a/spring-web/src/test/java/org/springframework/web/context/support/ServletContextResourceTests.java b/spring-web/src/test/java/org/springframework/web/context/support/ServletContextResourceTests.java new file mode 100644 index 0000000000..71333f5aab --- /dev/null +++ b/spring-web/src/test/java/org/springframework/web/context/support/ServletContextResourceTests.java @@ -0,0 +1,91 @@ +/* + * Copyright 2002-2022 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.web.context.support; + +import java.io.IOException; + +import org.junit.jupiter.api.Test; + +import org.springframework.core.io.Resource; +import org.springframework.web.testfixture.servlet.MockServletContext; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Tests for {@link ServletContextResource}. + * + * @author Chris Beams + * @author Brian Clozel + */ +public class ServletContextResourceTests { + + private static final String TEST_RESOURCE_PATH = "org/springframework/web/context/support/resource.txt"; + + private final MockServletContext servletContext = new MockServletContext(); + + @Test + void resourceShouldHaveExpectedProperties() throws IOException { + Resource resource = new ServletContextResource(this.servletContext, TEST_RESOURCE_PATH); + + assertThat(resource.getFile()).isNotNull(); + assertThat(resource.exists()).isTrue(); + assertThat(resource.isFile()).isTrue(); + assertThat(resource.getFilename()).isEqualTo("resource.txt"); + assertThat(resource.getURL().getFile().endsWith("resource.txt")).isTrue(); + } + + @Test + void relativeResourcesShouldHaveExpectedProperties() throws IOException { + Resource resource = new ServletContextResource(this.servletContext, TEST_RESOURCE_PATH); + Resource relative1 = resource.createRelative("relative.txt"); + assertThat(relative1.getFilename()).isEqualTo("relative.txt"); + assertThat(relative1.getURL().getFile().endsWith("relative.txt")).isTrue(); + assertThat(relative1.exists()).isTrue(); + + Resource relative2 = resource.createRelative("folder/other.txt"); + assertThat(relative2.getFilename()).isEqualTo("other.txt"); + assertThat(relative2.getURL().getFile().endsWith("other.txt")).isTrue(); + assertThat(relative2.exists()).isTrue(); + } + + @Test + void resourceWithDotPathShouldBeEqual() { + Resource resource = new ServletContextResource(this.servletContext, TEST_RESOURCE_PATH); + assertThat(new ServletContextResource(servletContext, "org/springframework/web/context/../context/support/./resource.txt")).isEqualTo(resource); + } + + @Test + void resourceWithRelativePathShouldBeEqual() throws IOException { + Resource resource = new ServletContextResource(this.servletContext, "dir/"); + Resource relative = resource.createRelative("subdir"); + assertThat(relative).isEqualTo(new ServletContextResource(this.servletContext, "dir/subdir")); + } + + @Test + void missingResourceShouldHaveExpectedProperties() { + MockServletContext context = mock(MockServletContext.class); + given(context.getRealPath(eq("/org/springframework/web/context/support/missing.txt"))) + .willReturn(this.servletContext.getRealPath("org/springframework/web/context/support/") + "missing.txt"); + Resource missing = new ServletContextResource(context, "org/springframework/web/context/support/missing.txt"); + + assertThat(missing.exists()).isFalse(); + assertThat(missing.isFile()).isFalse(); + } +} diff --git a/spring-web/src/test/resources/org/springframework/web/context/support/folder/other.txt b/spring-web/src/test/resources/org/springframework/web/context/support/folder/other.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/spring-web/src/test/resources/org/springframework/web/context/support/relative.txt b/spring-web/src/test/resources/org/springframework/web/context/support/relative.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/spring-web/src/test/resources/org/springframework/web/context/support/resource.txt b/spring-web/src/test/resources/org/springframework/web/context/support/resource.txt new file mode 100644 index 0000000000..e69de29bb2