This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push: new 704409d602 Fix BZ 69717 - expand test cases 704409d602 is described below commit 704409d602d7ba8685f6a1c722304549d2a98daa Author: Mark Thomas <ma...@apache.org> AuthorDate: Fri Jun 20 14:22:24 2025 +0100 Fix BZ 69717 - expand test cases --- .../catalina/webresources/AbstractResourceSet.java | 6 ++- .../webresources/AbstractTestResourceSet.java | 61 +++++++++++++--------- .../webresources/AbstractTestResourceSetMount.java | 5 ++ ...a => AbstractTestResourceSetMountTrailing.java} | 11 ++-- .../webresources/TestDirResourceSetMount.java | 2 +- ...t.java => TestDirResourceSetMountTrailing.java} | 4 +- ...t.java => TestJarResourceSetMountTrailing.java} | 41 +++------------ webapps/docs/changelog.xml | 10 ++-- 8 files changed, 70 insertions(+), 70 deletions(-) diff --git a/java/org/apache/catalina/webresources/AbstractResourceSet.java b/java/org/apache/catalina/webresources/AbstractResourceSet.java index 31d87d787d..0a0f01e720 100644 --- a/java/org/apache/catalina/webresources/AbstractResourceSet.java +++ b/java/org/apache/catalina/webresources/AbstractResourceSet.java @@ -71,7 +71,11 @@ public abstract class AbstractResourceSet extends LifecycleBase implements WebRe public final void setWebAppMount(String webAppMount) { checkPath(webAppMount); - // Optimise internal processing + /* + * Originally, only "/" was changed to "" to allow some optimisations. The fix for CVE-2025-49125 means that + * mounted WebResourceSets will break if webAppMount ends in '/'. So now the trailing "/" is removed in all + * cases. + */ if (webAppMount.endsWith("/")) { this.webAppMount = webAppMount.substring(0, webAppMount.length() - 1); } else { diff --git a/test/org/apache/catalina/webresources/AbstractTestResourceSet.java b/test/org/apache/catalina/webresources/AbstractTestResourceSet.java index 7e07ef1e26..373c15301d 100644 --- a/test/org/apache/catalina/webresources/AbstractTestResourceSet.java +++ b/test/org/apache/catalina/webresources/AbstractTestResourceSet.java @@ -44,6 +44,10 @@ public abstract class AbstractTestResourceSet { return ""; } + public String getMountPath() { + return ""; + } + public abstract File getBaseDir(); @Before @@ -77,23 +81,27 @@ public abstract class AbstractTestResourceSet { private void doTestGetResourceRoot(boolean slash) { - String mount = getMount(); - if (!slash && mount.length() == 0) { + String mountPath = getMountPath(); + if (!slash && mountPath.length() == 0) { return; } - mount = mount + (slash ? "/" : ""); + String path = mountPath + (slash ? "/" : ""); - WebResource webResource = resourceRoot.getResource(mount); + WebResource webResource = resourceRoot.getResource(path); Assert.assertTrue(webResource.isDirectory()); - String expected; - if (getMount().length() > 0) { - expected = getMount().substring(1); + String expectedName; + if (getMountPath().length() > 0) { + expectedName = getMountPath().substring(1); } else { - expected = ""; + expectedName = ""; + } + String expectedPath = path; + if (!path.endsWith("/")) { + expectedPath += "/"; } - Assert.assertEquals(expected, webResource.getName()); - Assert.assertEquals(mount + (!slash ? "/" : ""), webResource.getWebappPath()); + Assert.assertEquals(expectedName, webResource.getName()); + Assert.assertEquals(expectedPath, webResource.getWebappPath()); } @Test @@ -101,7 +109,7 @@ public abstract class AbstractTestResourceSet { WebResource webResource = resourceRoot.getResource(getMount() + "/d1"); Assert.assertTrue(webResource.isDirectory()); Assert.assertEquals("d1", webResource.getName()); - Assert.assertEquals(getMount() + "/d1/", webResource.getWebappPath()); + Assert.assertEquals(getMountPath() + "/d1/", webResource.getWebappPath()); Assert.assertEquals(-1, webResource.getContentLength()); Assert.assertNull(webResource.getContent()); Assert.assertNull(webResource.getInputStream()); @@ -112,7 +120,7 @@ public abstract class AbstractTestResourceSet { WebResource webResource = resourceRoot.getResource(getMount() + "/d1/"); Assert.assertTrue(webResource.isDirectory()); Assert.assertEquals("d1", webResource.getName()); - Assert.assertEquals(getMount() + "/d1/", webResource.getWebappPath()); + Assert.assertEquals(getMountPath() + "/d1/", webResource.getWebappPath()); Assert.assertEquals(-1, webResource.getContentLength()); Assert.assertNull(webResource.getContent()); Assert.assertNull(webResource.getInputStream()); @@ -120,14 +128,15 @@ public abstract class AbstractTestResourceSet { @Test public final void testGetResourceDirWithoutLeadingFileSeperator() { - String mount = getMount(); - if (mount.isEmpty()) { + // Use mount path for this test as the file separator needs to be missing + String mountPath = getMountPath(); + if (mountPath.isEmpty()) { // Test is only meaningful when resource is mounted below web application root. return; } - WebResource webResource = resourceRoot.getResource(mount + "d1"); + WebResource webResource = resourceRoot.getResource(mountPath + "d1"); Assert.assertFalse(webResource.exists()); - Assert.assertEquals(mount + "d1", webResource.getWebappPath()); + Assert.assertEquals(getMountPath() + "d1", webResource.getWebappPath()); } @Test @@ -137,7 +146,7 @@ public abstract class AbstractTestResourceSet { Assert.assertTrue(webResource.isFile()); Assert.assertEquals("d1-f1.txt", webResource.getName()); Assert.assertEquals( - getMount() + "/d1/d1-f1.txt", webResource.getWebappPath()); + getMountPath() + "/d1/d1-f1.txt", webResource.getWebappPath()); Assert.assertEquals(0, webResource.getContentLength()); Assert.assertEquals(0, webResource.getContent().length); Assert.assertNotNull(webResource.getInputStream()); @@ -293,18 +302,18 @@ public abstract class AbstractTestResourceSet { Set<String> results = resourceRoot.listWebAppPaths(mount + (slash ? "/" : "")); Set<String> expected = new HashSet<>(); - expected.add(getMount() + "/d1/"); - expected.add(getMount() + "/d2/"); - expected.add(getMount() + "/f1.txt"); - expected.add(getMount() + "/f2.txt"); + expected.add(getMountPath() + "/d1/"); + expected.add(getMountPath() + "/d2/"); + expected.add(getMountPath() + "/f1.txt"); + expected.add(getMountPath() + "/f2.txt"); // Directories created by Subversion 1.6 and earlier clients Set<String> optional = new HashSet<>(); - optional.add(getMount() + "/.svn/"); + optional.add(getMountPath() + "/.svn/"); // Files visible in some tests only - optional.add(getMount() + "/.ignore-me.txt"); + optional.add(getMountPath() + "/.ignore-me.txt"); // Files visible in some configurations only - optional.add(getMount() + "/META-INF/"); + optional.add(getMountPath() + "/META-INF/"); for (String result : results) { Assert.assertTrue(result, @@ -318,7 +327,7 @@ public abstract class AbstractTestResourceSet { Set<String> results = resourceRoot.listWebAppPaths(getMount() + "/d1"); Set<String> expected = new HashSet<>(); - expected.add(getMount() + "/d1/d1-f1.txt"); + expected.add(getMountPath() + "/d1/d1-f1.txt"); // Directories created by Subversion 1.6 and earlier clients Set<String> optional = new HashSet<>(); @@ -338,7 +347,7 @@ public abstract class AbstractTestResourceSet { Set<String> results = resourceRoot.listWebAppPaths(getMount() + "/d1/"); Set<String> expected = new HashSet<>(); - expected.add(getMount() + "/d1/d1-f1.txt"); + expected.add(getMountPath() + "/d1/d1-f1.txt"); // Directories created by Subversion 1.6 and earlier clients Set<String> optional = new HashSet<>(); diff --git a/test/org/apache/catalina/webresources/AbstractTestResourceSetMount.java b/test/org/apache/catalina/webresources/AbstractTestResourceSetMount.java index 3b84d67004..e46962804b 100644 --- a/test/org/apache/catalina/webresources/AbstractTestResourceSetMount.java +++ b/test/org/apache/catalina/webresources/AbstractTestResourceSetMount.java @@ -33,6 +33,11 @@ public abstract class AbstractTestResourceSetMount return "/mount"; } + @Override + public final String getMountPath() { + return "/mount"; + } + @Test public final void testGetResourceAbove() { WebResource webResource = resourceRoot.getResource("/"); diff --git a/test/org/apache/catalina/webresources/AbstractTestResourceSetMount.java b/test/org/apache/catalina/webresources/AbstractTestResourceSetMountTrailing.java similarity index 88% copy from test/org/apache/catalina/webresources/AbstractTestResourceSetMount.java copy to test/org/apache/catalina/webresources/AbstractTestResourceSetMountTrailing.java index 3b84d67004..8d674b8a29 100644 --- a/test/org/apache/catalina/webresources/AbstractTestResourceSetMount.java +++ b/test/org/apache/catalina/webresources/AbstractTestResourceSetMountTrailing.java @@ -25,11 +25,16 @@ import org.junit.Test; import org.apache.catalina.WebResource; -public abstract class AbstractTestResourceSetMount +public abstract class AbstractTestResourceSetMountTrailing extends AbstractTestResourceSet { @Override public final String getMount() { + return "/mount/"; + } + + @Override + public final String getMountPath() { return "/mount"; } @@ -45,7 +50,7 @@ public abstract class AbstractTestResourceSetMount Assert.assertNotNull(results); Assert.assertEquals(1, results.length); - Assert.assertEquals(getMount().substring(1), results[0]); + Assert.assertEquals(getMountPath().substring(1), results[0]); } @Test @@ -54,7 +59,7 @@ public abstract class AbstractTestResourceSetMount Assert.assertNotNull(results); Assert.assertEquals(1, results.size()); - Assert.assertTrue(results.contains(getMount() + "/")); + Assert.assertTrue(results.contains(getMountPath() + "/")); } @Test diff --git a/test/org/apache/catalina/webresources/TestDirResourceSetMount.java b/test/org/apache/catalina/webresources/TestDirResourceSetMount.java index fee86d67a3..9282669cb5 100644 --- a/test/org/apache/catalina/webresources/TestDirResourceSetMount.java +++ b/test/org/apache/catalina/webresources/TestDirResourceSetMount.java @@ -52,7 +52,7 @@ public class TestDirResourceSetMount extends AbstractTestResourceSetMount { public WebResourceRoot getWebResourceRoot() { TesterWebResourceRoot root = new TesterWebResourceRoot(); WebResourceSet webResourceSet = - new DirResourceSet(new TesterWebResourceRoot(), getMount() + "/", + new DirResourceSet(new TesterWebResourceRoot(), getMount(), getBaseDir().getAbsolutePath(), "/"); root.setMainResources(webResourceSet); return root; diff --git a/test/org/apache/catalina/webresources/TestDirResourceSetMount.java b/test/org/apache/catalina/webresources/TestDirResourceSetMountTrailing.java similarity index 96% copy from test/org/apache/catalina/webresources/TestDirResourceSetMount.java copy to test/org/apache/catalina/webresources/TestDirResourceSetMountTrailing.java index fee86d67a3..773e54e637 100644 --- a/test/org/apache/catalina/webresources/TestDirResourceSetMount.java +++ b/test/org/apache/catalina/webresources/TestDirResourceSetMountTrailing.java @@ -30,7 +30,7 @@ import org.apache.catalina.WebResourceSet; import org.apache.catalina.startup.ExpandWar; import org.apache.catalina.startup.TomcatBaseTest; -public class TestDirResourceSetMount extends AbstractTestResourceSetMount { +public class TestDirResourceSetMountTrailing extends AbstractTestResourceSetMountTrailing { private static Path tempDir; private static File dir1; @@ -52,7 +52,7 @@ public class TestDirResourceSetMount extends AbstractTestResourceSetMount { public WebResourceRoot getWebResourceRoot() { TesterWebResourceRoot root = new TesterWebResourceRoot(); WebResourceSet webResourceSet = - new DirResourceSet(new TesterWebResourceRoot(), getMount() + "/", + new DirResourceSet(new TesterWebResourceRoot(), getMount(), getBaseDir().getAbsolutePath(), "/"); root.setMainResources(webResourceSet); return root; diff --git a/test/org/apache/catalina/webresources/TestDirResourceSetMount.java b/test/org/apache/catalina/webresources/TestJarResourceSetMountTrailing.java similarity index 57% copy from test/org/apache/catalina/webresources/TestDirResourceSetMount.java copy to test/org/apache/catalina/webresources/TestJarResourceSetMountTrailing.java index fee86d67a3..f64b4a7ca3 100644 --- a/test/org/apache/catalina/webresources/TestDirResourceSetMount.java +++ b/test/org/apache/catalina/webresources/TestJarResourceSetMountTrailing.java @@ -17,69 +17,44 @@ package org.apache.catalina.webresources; import java.io.File; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.attribute.FileAttribute; - -import org.junit.AfterClass; -import org.junit.BeforeClass; import org.apache.catalina.WebResourceRoot; import org.apache.catalina.WebResourceSet; -import org.apache.catalina.startup.ExpandWar; -import org.apache.catalina.startup.TomcatBaseTest; - -public class TestDirResourceSetMount extends AbstractTestResourceSetMount { - - private static Path tempDir; - private static File dir1; - - @BeforeClass - public static void before() throws IOException { - tempDir = Files.createTempDirectory("test", new FileAttribute[0]); - dir1 = new File(tempDir.toFile(), "dir1"); - TomcatBaseTest.recursiveCopy(new File("test/webresources/dir1").toPath(), dir1.toPath()); - } - - @AfterClass - public static void after() { - ExpandWar.delete(tempDir.toFile()); - } +public class TestJarResourceSetMountTrailing extends AbstractTestResourceSetMount { @Override public WebResourceRoot getWebResourceRoot() { + File f = new File("test/webresources/dir1.jar"); TesterWebResourceRoot root = new TesterWebResourceRoot(); WebResourceSet webResourceSet = - new DirResourceSet(new TesterWebResourceRoot(), getMount() + "/", - getBaseDir().getAbsolutePath(), "/"); + new JarResourceSet(root, getMount(), f.getAbsolutePath(), "/"); root.setMainResources(webResourceSet); return root; } @Override protected boolean isWritable() { - return true; + return false; } @Override public File getBaseDir() { - return dir1; + return new File("test/webresources"); } @Override protected String getNewDirName() { - return "test-dir-03"; + return "test-dir-10"; } @Override protected String getNewFileNameNull() { - return "test-null-03"; + return "test-null-10"; } @Override protected String getNewFileName() { - return "test-file-03"; + return "test-file-10"; } } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index bec9650d9f..af76271bbd 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -175,6 +175,12 @@ length is known, no content has been written and a <code>Writer</code> is being used. (markt) </fix> + <fix> + <bug>69717</bug>: Correct a regression in the fix for CVE-2025-49125 + that prevented access to PreResources and PostResources when mounted + below the web application root with a path that was terminated with a + file separator. (remm/markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> @@ -206,10 +212,6 @@ Fix JMX value for <code>keepAliveCount</code> on the endpoint. Also add the value of <code>useVirtualThreads</code> in JMX. (remm) </fix> - <fix> - <bug>69717</bug>: Allow trailing slash for <code>webAppMount</code> - path in <code>Resources</code>, for compatibility reasons. (remm) - </fix> </changelog> </subsection> <subsection name="Jasper"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org