diff --git a/TODO.md b/TODO.md index c289a1048bc6..f53c6849255e 100644 --- a/TODO.md +++ b/TODO.md @@ -369,14 +369,13 @@ These are bugs, correctness issues, or missing functionality that may affect pro --- -### 32. Jasper TLD Validation (2 items) +### 32. Jasper TLD Validation (1 item) | # | File:Line | Description | Fix Idea | Effort | Difficulty | |---|-----------|-------------|----------|--------|------------| -| 32.1 | `TagLibraryInfoImpl.java:193` | Duplicate function name validation should move to parsing stage | Add duplicate name detection in the TLD parser (`TaglibXmlParser`) before the `TagLibraryInfoImpl` is constructed. | 1 day | Medium | | 32.2 | `TagLibraryInfoImpl.java:231` | URL resolution logic for TLD resource paths looks incorrect | Audit the URI resolution logic against JSP spec section 7.3.6.2. Fix any deviations. | 1-2 days | Medium | -**Total estimated effort: 2-3 days, Medium difficulty** +**Total estimated effort: 1-2 days, Medium difficulty** --- diff --git a/java/org/apache/jasper/compiler/TagLibraryInfoImpl.java b/java/org/apache/jasper/compiler/TagLibraryInfoImpl.java index 02aae498a43b..a33044c00833 100644 --- a/java/org/apache/jasper/compiler/TagLibraryInfoImpl.java +++ b/java/org/apache/jasper/compiler/TagLibraryInfoImpl.java @@ -26,10 +26,8 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import jakarta.servlet.jsp.tagext.FunctionInfo; import jakarta.servlet.jsp.tagext.PageData; @@ -188,15 +186,7 @@ public String toString() { tagFileInfos.add(createTagFileInfo(tagFileXml, jar)); } - Set names = new HashSet<>(); List functionInfos = taglibXml.getFunctions(); - // TODO Move this validation to the parsing stage - for (FunctionInfo functionInfo : functionInfos) { - String name = functionInfo.getName(); - if (!names.add(name)) { - err.jspError("jsp.error.tld.fn.duplicate.name", name, uri); - } - } if (tlibversion == null) { err.jspError("jsp.error.tld.mandatory.element.missing", "tlib-version", uri); diff --git a/java/org/apache/jasper/resources/LocalStrings.properties b/java/org/apache/jasper/resources/LocalStrings.properties index c9d4215a35f2..381c2e23c34a 100644 --- a/java/org/apache/jasper/resources/LocalStrings.properties +++ b/java/org/apache/jasper/resources/LocalStrings.properties @@ -227,7 +227,6 @@ jsp.error.taglibDirective.uriInvalid=The URI provided for a tag library [{0}] is jsp.error.tei.invalid.attributes=Validation error messages from TagExtraInfo for [{0}] jsp.error.teiclass.instantiation=Failed to load or instantiate TagExtraInfo class: [{0}] jsp.error.text.has_subelement=<jsp:text> must not have any subelements -jsp.error.tld.fn.duplicate.name=Duplicate function name [{0}] in tag library [{1}] jsp.error.tld.fn.invalid.signature=Invalid syntax for function signature in TLD. Tag Library: [{0}], Function: [{1}] jsp.error.tld.invalid_tld_file=Invalid tld file: [{0}], see JSP specification section 7.3.1 for more details jsp.error.tld.mandatory.element.missing=Mandatory TLD element [{0}] missing or empty in TLD [{1}] diff --git a/java/org/apache/tomcat/util/descriptor/tld/LocalStrings.properties b/java/org/apache/tomcat/util/descriptor/tld/LocalStrings.properties index 36705cc726af..92e262f87d54 100644 --- a/java/org/apache/tomcat/util/descriptor/tld/LocalStrings.properties +++ b/java/org/apache/tomcat/util/descriptor/tld/LocalStrings.properties @@ -14,3 +14,5 @@ # limitations under the License. implicitTldRule.elementNotAllowed=The element [{0}] is not permitted in an implicit.tld file + +taglibXml.duplicateFunction=Duplicate function name [{0}] in tag library diff --git a/java/org/apache/tomcat/util/descriptor/tld/TaglibXml.java b/java/org/apache/tomcat/util/descriptor/tld/TaglibXml.java index 89e6fb49cc17..51d873d4027a 100644 --- a/java/org/apache/tomcat/util/descriptor/tld/TaglibXml.java +++ b/java/org/apache/tomcat/util/descriptor/tld/TaglibXml.java @@ -17,10 +17,14 @@ package org.apache.tomcat.util.descriptor.tld; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import jakarta.servlet.jsp.tagext.FunctionInfo; +import org.apache.tomcat.util.res.StringManager; + /** * Common representation of a Tag Library Descriptor (TLD) XML file. *

@@ -29,6 +33,9 @@ * contain the uri and prefix values used by a JSP to reference this tag library. */ public class TaglibXml { + + private static final StringManager sm = StringManager.getManager(TaglibXml.class); + /** * Constructs a new TaglibXml. */ @@ -85,6 +92,11 @@ public TaglibXml() { */ private final List functions = new ArrayList<>(); + /** + * The function names used to detect duplicate definitions. + */ + private final Set functionNames = new HashSet<>(); + /** * Returns the tag library version. * @return the library version @@ -234,8 +246,12 @@ public List getListeners() { * @param name the function name * @param klass the function class * @param signature the function signature + * @throws IllegalArgumentException if a function with the same name has already been added */ public void addFunction(String name, String klass, String signature) { + if (!functionNames.add(name)) { + throw new IllegalArgumentException(sm.getString("taglibXml.duplicateFunction", name)); + } functions.add(new FunctionInfo(name, klass, signature)); } diff --git a/test/org/apache/tomcat/util/descriptor/tld/TestTldParser.java b/test/org/apache/tomcat/util/descriptor/tld/TestTldParser.java index d04d691ae772..1d0faeb25a13 100644 --- a/test/org/apache/tomcat/util/descriptor/tld/TestTldParser.java +++ b/test/org/apache/tomcat/util/descriptor/tld/TestTldParser.java @@ -164,6 +164,14 @@ public void testListener() throws Exception { Assert.assertEquals("org.apache.catalina.core.TesterTldListener", listeners.get(0)); } + @Test + public void testDuplicateFunctionName() { + parser = new TldParser(true, false, new TldRuleSet(), true); + SAXException exception = + Assert.assertThrows(SAXException.class, () -> parse("test/tld/duplicate-function.tld")); + Assert.assertTrue(exception.getMessage(), exception.getMessage().contains("Duplicate function name [trim]")); + } + private TaglibXml parse(String pathname) throws IOException, SAXException { File file = new File(pathname); TldResourcePath path = new TldResourcePath(file.toURI().toURL(), null); diff --git a/test/tld/duplicate-function.tld b/test/tld/duplicate-function.tld new file mode 100644 index 000000000000..2484baedce86 --- /dev/null +++ b/test/tld/duplicate-function.tld @@ -0,0 +1,40 @@ + + + + 1.0 + duplicate-function + + + trim + org.apache.el.TesterFunctions + + java.lang.String trim(java.lang.String) + + + + trim + org.apache.el.TesterFunctions + + java.lang.String trim(java.lang.String) + + + diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index b9846801cdd5..e510bc2555ee 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -372,6 +372,11 @@ Add support for java.util.Optional to the empty operator. (markt) + + Detect duplicate function names while parsing a tag library descriptor + rather than when constructing its TagLibraryInfo. + (sainadh777) +