This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 564d2129a9ee4630920e48db002000c3a6137401 Author: opencode <[email protected]> AuthorDate: Mon Oct 12 19:25:43 2026 +0200 Fix archive index pre-check for mounted archive resource sets The bloom filter pre-check in AbstractArchiveResourceSet.getResource() was queried with the web application path whenever the resource set had a non-empty internal path. That embedded the mount point in the middle of the hashed string, so, once the filter had been built, every lookup against an archive resource set mounted below the web application root with an internal path was reported as definitely not present and returned a 404. The pre-check now uses the JAR relative path, which is what the filter is keyed on. While reproducing the issue, it also emerged that a request for the mount point itself of a resource set with an internal path resolved to the archive entry mapped to that internal path. The name of that entry is reported as the empty string for internal path resource sets, so the mount point did not match the resource set contract, and archives without an explicit directory entry for the internal path did not resolve their own mount point at all. A request for the mount point now always resolves to the archive root, as was already the case when the internal path was empty. Add TestJarResourceSetInternalMount which exercises the resource set contract for an archive resource set with both a mount point and an internal path, including lookups made after the bloom filter has been built. --- .../webresources/AbstractArchiveResourceSet.java | 88 ++++++++++++---------- .../TestJarResourceSetInternalMount.java | 88 ++++++++++++++++++++++ 2 files changed, 136 insertions(+), 40 deletions(-) diff --git a/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java b/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java index 05220e2f13..0da5a125d6 100644 --- a/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java +++ b/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java @@ -250,15 +250,6 @@ public abstract class AbstractArchiveResourceSet extends AbstractResourceSet { String webAppMount = getWebAppMount(); WebResourceRoot root = getRoot(); - /* - * If jarContents reports that this resource definitely does not contain the path, we can end this method and - * move on to the next jar. - */ - if (jarContents != null && !jarContents - .mightContainResource(getInternalPath().isEmpty() ? path : getInternalPath() + path, webAppMount)) { - return new EmptyResource(root, path); - } - /* * Implementation notes * @@ -275,49 +266,66 @@ public abstract class AbstractArchiveResourceSet extends AbstractResourceSet { // an empty resource for requests outside of the mount point. if (isPathMounted(path, webAppMount)) { - String pathInJar = getInternalPath() + path.substring(webAppMount.length()); - // Always strip off the leading '/' to get the JAR path - if (!pathInJar.isEmpty() && pathInJar.charAt(0) == '/') { - pathInJar = pathInJar.substring(1); - } - if (pathInJar.isEmpty()) { - // Special case + String pathSuffix = path.substring(webAppMount.length()); + + // If the requested path is the mount point itself, the requested + // resource is the root of the path in the archive that this set + // maps to the mount point. Always treat it as a directory + // resource, represented by the archive root rather than by a + // (possibly absent) directory entry in the archive. + if (pathSuffix.isEmpty() || pathSuffix.equals("/")) { // This is a directory resource so the path must end with / if (!path.endsWith("/")) { path = path + "/"; } return new JarResourceRoot(root, new File(getBase()), baseUrlString, path); + } + + String pathInJar = getInternalPath() + pathSuffix; + // Always strip off the leading '/' to get the JAR path + if (!pathInJar.isEmpty() && pathInJar.charAt(0) == '/') { + pathInJar = pathInJar.substring(1); + } + + /* + * If jarContents reports that this resource definitely does not contain the path, we can end this method + * and move on to the next jar. The bloom filter is keyed on the entry names in the archive, so the + * pre-check must use the JAR relative path. Hashing the web application path (with the mount point + * embedded in it) would result in false negatives. + */ + if (jarContents != null && !jarContents.mightContainResource(pathInJar, "")) { + return new EmptyResource(root, path); + } + + JarEntry jarEntry = null; + if (isMultiRelease()) { + // Calls JarFile.getJarEntry() which is multi-release aware + jarEntry = getArchiveEntry(pathInJar); } else { - JarEntry jarEntry = null; - if (isMultiRelease()) { - // Calls JarFile.getJarEntry() which is multi-release aware - jarEntry = getArchiveEntry(pathInJar); - } else { - Map<String,JarEntry> jarEntries = getArchiveEntries(true); - if (!(pathInJar.charAt(pathInJar.length() - 1) == '/')) { - if (jarEntries == null) { - jarEntry = getArchiveEntry(pathInJar + '/'); - } else { - jarEntry = jarEntries.get(pathInJar + '/'); - } - if (jarEntry != null) { - path = path + '/'; - } + Map<String,JarEntry> jarEntries = getArchiveEntries(true); + if (!(pathInJar.charAt(pathInJar.length() - 1) == '/')) { + if (jarEntries == null) { + jarEntry = getArchiveEntry(pathInJar + '/'); + } else { + jarEntry = jarEntries.get(pathInJar + '/'); } - if (jarEntry == null) { - if (jarEntries == null) { - jarEntry = getArchiveEntry(pathInJar); - } else { - jarEntry = jarEntries.get(pathInJar); - } + if (jarEntry != null) { + path = path + '/'; } } if (jarEntry == null) { - return new EmptyResource(root, path); - } else { - return createArchiveResource(jarEntry, path, getManifest()); + if (jarEntries == null) { + jarEntry = getArchiveEntry(pathInJar); + } else { + jarEntry = jarEntries.get(pathInJar); + } } } + if (jarEntry == null) { + return new EmptyResource(root, path); + } else { + return createArchiveResource(jarEntry, path, getManifest()); + } } else { return new EmptyResource(root, path); } diff --git a/test/org/apache/catalina/webresources/TestJarResourceSetInternalMount.java b/test/org/apache/catalina/webresources/TestJarResourceSetInternalMount.java new file mode 100644 index 0000000000..2d890b5fdc --- /dev/null +++ b/test/org/apache/catalina/webresources/TestJarResourceSetInternalMount.java @@ -0,0 +1,88 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 + * + * http://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.apache.catalina.webresources; + +import java.io.File; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.WebResource; +import org.apache.catalina.WebResourceRoot; +import org.apache.catalina.WebResourceSet; + +public class TestJarResourceSetInternalMount extends AbstractTestResourceSetMount { + + @Override + public WebResourceRoot getWebResourceRoot() { + File f = new File("test/webresources/dir1-internal.jar"); + TesterWebResourceRoot root = new TesterWebResourceRoot(); + WebResourceSet webResourceSet = + new JarResourceSet(root, getMount(), f.getAbsolutePath(), "/dir1"); + root.setMainResources(webResourceSet); + return root; + } + + @Override + protected boolean isWritable() { + return false; + } + + @Override + public File getBaseDir() { + return new File("test/webresources"); + } + + @Override + protected String getNewDirName() { + return "test-dir-12"; + } + + @Override + protected String getNewFileNameNull() { + return "test-null-12"; + } + + @Override + protected String getNewFileName() { + return "test-file-12"; + } + + @Test + public void testGetResourceAfterBloomFilterBuilt() { + // Regression test: resources must still be found once the archive + // (and therefore the bloom filter) has been opened by a previous + // read, even when the resource set has both a non-empty internal + // path and a non-root mount point. + WebResource file = resourceRoot.getResource(getMount() + "/d1/d1-f1.txt"); + Assert.assertTrue(file.isFile()); + // Reading the content opens the archive which, along with the + // preceding lookup itself, builds the bloom filter for the archive + // index. Subsequent lookups are then checked against that filter. + Assert.assertNotNull(file.getContent()); + + WebResource fileAfterFilterBuilt = + resourceRoot.getResource(getMount() + "/d1/d1-f1.txt"); + Assert.assertTrue(fileAfterFilterBuilt.isFile()); + + WebResource dirAfterFilterBuilt = resourceRoot.getResource(getMount() + "/d1"); + Assert.assertTrue(dirAfterFilterBuilt.isDirectory()); + + WebResource mountRootAfterFilterBuilt = resourceRoot.getResource(getMount()); + Assert.assertTrue(mountRootAfterFilterBuilt.isDirectory()); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
