Validate duplicate TLD functions during parsing

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**
 
 ---
 

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<String> names = new HashSet<>();
             List<FunctionInfo> 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);

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}]

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

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.
  * <p>
@@ -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<FunctionInfo> functions = new ArrayList<>();
 
+    /**
+     * The function names used to detect duplicate definitions.
+     */
+    private final Set<String> functionNames = new HashSet<>();
+
     /**
      * Returns the tag library version.
      * @return the library version
@@ -234,8 +246,12 @@ public List<String> 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));
     }
 

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);

test/tld/duplicate-function.tld

@@ -0,0 +1,40 @@
+<?xml version="1.0" encoding="ISO-8859-1" ?>
+<!--
+  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.
+-->
+<taglib xmlns="https://jakarta.ee/xml/ns/jakartaee"
+      xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+      xsi:schemaLocation="https://jakarta.ee/xml/ns/jakartaee
+      https://jakarta.ee/xml/ns/jakartaee/web-jsptaglibrary_4_0.xsd"
+      version="4.0">
+  <tlib-version>1.0</tlib-version>
+  <short-name>duplicate-function</short-name>
+
+  <function>
+    <name>trim</name>
+    <function-class>org.apache.el.TesterFunctions</function-class>
+    <function-signature>
+      java.lang.String trim(java.lang.String)
+    </function-signature>
+  </function>
+  <function>
+    <name>trim</name>
+    <function-class>org.apache.el.TesterFunctions</function-class>
+    <function-signature>
+      java.lang.String trim(java.lang.String)
+    </function-signature>
+  </function>
+</taglib>

webapps/docs/changelog.xml

@@ -372,6 +372,11 @@
         Add support for <code>java.util.Optional</code> to the empty operator.
         (markt)
       </add>
+      <fix>
+        Detect duplicate function names while parsing a tag library descriptor
+        rather than when constructing its <code>TagLibraryInfo</code>.
+        (sainadh777)
+      </fix>
       <!-- Entries for backport and removal before 12.0.0-M1 below this line -->
     </changelog>
   </subsection>