This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 1420cd51b87d5ee1121b91e9bd54ece60f3a619d Author: opencode <[email protected]> AuthorDate: Wed Oct 14 13:10:41 2026 +0200 Reject a null URL in the TldResourcePath constructors TldResourcePath accepted a null URL without complaint and then failed later with a NullPointerException in toExternalForm(), openStream(), equals() or hashCode() when the value was first used. One caller made this reachable in normal operation. TldScanner resolved a TLD listed by ServletContext.getResourcePaths() with ServletContext.getResource() and passed the result straight to the constructor. The two calls are not atomic: a TLD removed from an exploded web application between them resolved to null, the resulting NullPointerException was thrown out of the scan and, since JasperInitializer.onStartup() only catches IOException and SAXException, it aborted context start-up with an opaque error. Both constructors now throw an IllegalArgumentException with a readable, localised message when the URL is null, mirroring the validation style used elsewhere in the utility packages. In addition, TldScanner.parseTld(String) now checks for the vanished resource and skips it with a warning, in the same way as scanJspConfig() handles a taglib-location in web.xml that does not exist, so the race becomes a startup warning rather than a failure. Tests: TestTldResourcePath covers the constructor rejection and the methods the invariant protects; TestTldScanner covers the scan-time warn-and-skip of a resource that disappeared. --- .../jasper/resources/LocalStrings.properties | 1 + java/org/apache/jasper/servlet/TldScanner.java | 16 ++++-- .../util/descriptor/tld/LocalStrings.properties | 1 + .../util/descriptor/tld/TldResourcePath.java | 11 ++++ test/org/apache/jasper/servlet/TestTldScanner.java | 30 +++++++++++ .../util/descriptor/tld/TestTldResourcePath.java | 63 ++++++++++++++++++++++ 6 files changed, 119 insertions(+), 3 deletions(-) diff --git a/java/org/apache/jasper/resources/LocalStrings.properties b/java/org/apache/jasper/resources/LocalStrings.properties index 6578bf4cf9..029dff2c2d 100644 --- a/java/org/apache/jasper/resources/LocalStrings.properties +++ b/java/org/apache/jasper/resources/LocalStrings.properties @@ -427,6 +427,7 @@ org.apache.jasper.compiler.ELParser.missingQuote=The expression [{0}] is not val org.apache.jasper.compiler.TldCache.servletContextNull=The provided ServletContext was null org.apache.jasper.servlet.JasperInitializer.onStartup=Initializing Jasper for context [{0}] org.apache.jasper.servlet.TldScanner.implicitTldInvalid=The implicit TLD at resource path [{0}] was ignored as it is not valid: {1} +org.apache.jasper.servlet.TldScanner.resourcePathDoesNotExist=Failed to process TLD at resource path [{0}]. The specified path does not exist. org.apache.jasper.servlet.TldScanner.webxmlAdd=Loading TLD for URI [{1}] from resource path [{0}] org.apache.jasper.servlet.TldScanner.webxmlFailPathDoesNotExist=Failed to process TLD with path [{0}] and URI [{1}]. The specified path does not exist. org.apache.jasper.servlet.TldScanner.webxmlSkip=Skipping load of TLD for URI [{1}] from resource path [{0}] as it has already been defined in <jsp-config> diff --git a/java/org/apache/jasper/servlet/TldScanner.java b/java/org/apache/jasper/servlet/TldScanner.java index 4d34525dba..cb04ac4a0f 100644 --- a/java/org/apache/jasper/servlet/TldScanner.java +++ b/java/org/apache/jasper/servlet/TldScanner.java @@ -269,15 +269,25 @@ public class TldScanner { } /** - * Parses a TLD from the given resource path string. + * Parses a TLD from the given resource path string. If the resource no + * longer exists - it may have been removed since it was listed by + * {@link ServletContext#getResourcePaths(String)} - the TLD is skipped and + * a warning is logged. * * @param resourcePath the resource path * @throws IOException if an I/O error occurs * @throws SAXException if a parsing error occurs */ protected void parseTld(String resourcePath) throws IOException, SAXException { - TldResourcePath tldResourcePath = new TldResourcePath(context.getResource(resourcePath), resourcePath); - parseTld(tldResourcePath); + URL url = context.getResource(resourcePath); + if (url == null) { + // The resource may have been removed after it was discovered by + // getResourcePaths(). Skip it rather than aborting the scan, as is + // done for a missing resource referenced by web.xml. + log.warn(Localizer.getMessage(MSG + ".resourcePathDoesNotExist", resourcePath)); + return; + } + parseTld(new TldResourcePath(url, resourcePath)); } /** diff --git a/java/org/apache/tomcat/util/descriptor/tld/LocalStrings.properties b/java/org/apache/tomcat/util/descriptor/tld/LocalStrings.properties index 5504dbfd79..41b6b67571 100644 --- a/java/org/apache/tomcat/util/descriptor/tld/LocalStrings.properties +++ b/java/org/apache/tomcat/util/descriptor/tld/LocalStrings.properties @@ -17,3 +17,4 @@ # To edit translations see: https://tomcat.apache.org/getinvolved.html#Translations implicitTldRule.elementNotAllowed=The element [{0}] is not permitted in an implicit.tld file +tldResourcePath.nullUrl=The URL may not be null diff --git a/java/org/apache/tomcat/util/descriptor/tld/TldResourcePath.java b/java/org/apache/tomcat/util/descriptor/tld/TldResourcePath.java index f0793dd7f2..c17c94c325 100644 --- a/java/org/apache/tomcat/util/descriptor/tld/TldResourcePath.java +++ b/java/org/apache/tomcat/util/descriptor/tld/TldResourcePath.java @@ -22,6 +22,7 @@ import java.net.URL; import java.util.Objects; import org.apache.tomcat.Jar; +import org.apache.tomcat.util.res.StringManager; import org.apache.tomcat.util.scan.JarFactory; import org.apache.tomcat.util.scan.ReferenceCountedJar; @@ -39,6 +40,9 @@ import org.apache.tomcat.util.scan.ReferenceCountedJar; * URL and entryName when using this implementation. */ public class TldResourcePath { + + private static final StringManager sm = StringManager.getManager(TldResourcePath.class); + private final URL url; private final String webappPath; private final String entryName; @@ -48,6 +52,8 @@ public class TldResourcePath { * * @param url the location of the TLD * @param webappPath the web application path, if any, of the TLD + * + * @throws IllegalArgumentException if the provided URL is null */ public TldResourcePath(URL url, String webappPath) { this(url, webappPath, null); @@ -59,8 +65,13 @@ public class TldResourcePath { * @param url the location of the JAR * @param webappPath the web application path, if any, of the JAR * @param entryName the name of the entry in the JAR + * + * @throws IllegalArgumentException if the provided URL is null */ public TldResourcePath(URL url, String webappPath, String entryName) { + if (url == null) { + throw new IllegalArgumentException(sm.getString("tldResourcePath.nullUrl")); + } this.url = url; this.webappPath = webappPath; this.entryName = entryName; diff --git a/test/org/apache/jasper/servlet/TestTldScanner.java b/test/org/apache/jasper/servlet/TestTldScanner.java index 669be2286f..ef5b8cda95 100644 --- a/test/org/apache/jasper/servlet/TestTldScanner.java +++ b/test/org/apache/jasper/servlet/TestTldScanner.java @@ -22,9 +22,13 @@ import java.lang.reflect.Field; import java.net.URL; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.logging.Level; + +import jakarta.servlet.ServletContext; import org.junit.Assert; import org.junit.Test; @@ -108,6 +112,32 @@ public class TestTldScanner extends TomcatBaseTest { } + @Test + public void testParseTldMissingResourceIsSkipped() throws Exception { + // A resource listed by getResourcePaths() may be gone by the time it + // is resolved with getResource() (e.g. a file removed during a + // development-mode scan). The scan must skip it with a warning rather + // than propagate a NullPointerException out of the constructor. + ServletContext mockContext = EasyMock.createMock(ServletContext.class); + EasyMock.expect(mockContext.getResourcePaths("/WEB-INF/")) + .andReturn(Collections.singleton("/WEB-INF/vanished.tld")).once(); + EasyMock.expect(mockContext.getResource("/WEB-INF/vanished.tld")).andReturn(null).once(); + EasyMock.replay(mockContext); + + TldScanner scanner = new TldScanner(mockContext, true, true, true); + try (LogCapture logCapture = attachLogCapture(Level.WARNING, TldScanner.class.getName())) { + scanner.scanResourcePaths("/WEB-INF/"); + Assert.assertTrue(logCapture.containsText( + "Failed to process TLD at resource path [/WEB-INF/vanished.tld]")); + } + + EasyMock.verify(mockContext); + // The vanished TLD contributed nothing. + Assert.assertEquals(0, scanner.getUriTldResourcePathMap().size()); + Assert.assertEquals(0, scanner.getTldResourcePathTaglibXmlMap().size()); + } + + private static void createImplicitTld(File appDir, String extraElements) throws Exception { File implicitTld = new File(appDir, "WEB-INF/tags/display/implicit.tld"); Files.createDirectories(implicitTld.getParentFile().toPath()); diff --git a/test/org/apache/tomcat/util/descriptor/tld/TestTldResourcePath.java b/test/org/apache/tomcat/util/descriptor/tld/TestTldResourcePath.java new file mode 100644 index 0000000000..d614b9452a --- /dev/null +++ b/test/org/apache/tomcat/util/descriptor/tld/TestTldResourcePath.java @@ -0,0 +1,63 @@ +/* + * 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.tomcat.util.descriptor.tld; + +import java.io.File; +import java.net.URL; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.tomcat.util.res.StringManager; + +public class TestTldResourcePath { + + @Test(expected = IllegalArgumentException.class) + public void testNullUrlRejectedTwoArg() { + new TldResourcePath(null, "/WEB-INF/tld/test.tld"); + } + + @Test(expected = IllegalArgumentException.class) + public void testNullUrlRejectedThreeArg() { + new TldResourcePath(null, "/WEB-INF/lib/test.jar", "META-INF/test.tld"); + } + + @Test + public void testNullUrlMessage() { + try { + new TldResourcePath(null, "/WEB-INF/tld/test.tld"); + Assert.fail("Expected an IllegalArgumentException"); + } catch (IllegalArgumentException e) { + String expected = StringManager.getManager(TldResourcePath.class) + .getString("tldResourcePath.nullUrl"); + Assert.assertEquals(expected, e.getMessage()); + } + } + + @Test + public void testNonNullUrlAccepted() throws Exception { + URL url = new File("/tmp/test.tld").toURI().toURL(); + TldResourcePath path = new TldResourcePath(url, "/WEB-INF/tld/test.tld"); + Assert.assertEquals(url, path.getUrl()); + Assert.assertEquals("/WEB-INF/tld/test.tld", path.getWebappPath()); + Assert.assertNull(path.getEntryName()); + // The methods the null URL check exists to protect. + Assert.assertEquals(url.toExternalForm(), path.toExternalForm()); + Assert.assertEquals(new TldResourcePath(url, "/WEB-INF/tld/test.tld"), path); + Assert.assertEquals(new TldResourcePath(url, "/WEB-INF/tld/test.tld").hashCode(), path.hashCode()); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
