Manually merge #535 - improve error message for duplicate fragments

Now lists the JARs that contain the duplicates. PR by Mads Rolsdorph

Author: markt-asf

java/org/apache/tomcat/util/descriptor/web/FragmentJarScannerCallback.java

@@ -136,7 +136,7 @@ private void addFragment(WebXml fragment, URL url) {
             // this name as having a duplicate so Tomcat can handle it
             // correctly when the fragments are being ordered.
             String duplicateName = fragment.getName();
-            fragments.get(duplicateName).setDuplicated(true);
+            fragments.get(duplicateName).addDuplicate(url.toString());
             // Rename the current fragment so it doesn't clash
             fragment.setName(url.toString());
         }

java/org/apache/tomcat/util/descriptor/web/LocalStrings.properties

@@ -32,7 +32,7 @@ webRuleSet.relativeOrderingCount=<ordering> element is limited to 1 occurrence
 
 webXml.duplicateEnvEntry=Duplicate env-entry name [{0}]
 webXml.duplicateFilter=Duplicate filter name [{0}]
-webXml.duplicateFragment=More than one fragment with the name [{0}] was found. This is not legal with relative ordering. See section 8.2.2 2c of the Servlet specification for details. Consider using absolute ordering.
+webXml.duplicateFragment=More than one fragment with the name [{0}] was found. This is not legal with relative ordering. See section 8.2.2 2c of the Servlet specification for details. Consider using absolute ordering. Duplicate fragments found in [{1}].
 webXml.duplicateMessageDestination=Duplicate message-destination name [{0}]
 webXml.duplicateMessageDestinationRef=Duplicate message-destination-ref name [{0}]
 webXml.duplicateResourceEnvRef=Duplicate resource-env-ref name [{0}]

java/org/apache/tomcat/util/descriptor/web/WebXml.java

@@ -84,12 +84,23 @@ public void setOverridable(boolean overridable) {
      * to know as the action that the specification requires (see 8.2.2 1.e and
      * 2.c) varies depending on the ordering method used.
      */
-    private boolean duplicated = false;
+    private final List<String> duplicates = new ArrayList<>();
     public boolean isDuplicated() {
-        return duplicated;
+        return !duplicates.isEmpty();
     }
+    @Deprecated
     public void setDuplicated(boolean duplicated) {
-        this.duplicated = duplicated;
+        if (duplicated) {
+            duplicates.add("unknown");
+        } else {
+            duplicates.clear();
+        }
+    }
+    public void addDuplicate(String duplicate) {
+        this.duplicates.add(duplicate);
+    }
+    public List<String> getDuplicates() {
+        return new ArrayList<>(this.duplicates);
     }
 
     /**
@@ -2264,8 +2275,10 @@ private Set<WebXml> orderWebFragments(Map<String,WebXml> fragments,
             // Stage 0. Check there were no fragments with duplicate names
             for (WebXml fragment : fragments.values()) {
                 if (fragment.isDuplicated()) {
+                    List<String> duplicates = fragment.getDuplicates();
+                    duplicates.add(0, fragment.getURL().toString());
                     throw new IllegalArgumentException(
-                            sm.getString("webXml.duplicateFragment", fragment.getName()));
+                            sm.getString("webXml.duplicateFragment", fragment.getName(), duplicates));
                 }
             }
             // Stage 1. Make all dependencies bi-directional - this makes the

test/org/apache/tomcat/util/descriptor/web/TestWebXmlOrdering.java

@@ -16,6 +16,8 @@
  */
 package org.apache.tomcat.util.descriptor.web;
 
+import java.net.MalformedURLException;
+import java.net.URL;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -348,6 +350,31 @@ public void testOrderWebFragmentsrelativeCircular2() {
         WebXml.orderWebFragments(app, fragments, null);
     }
 
+    @Test(expected=IllegalArgumentException.class)
+    public void testOrderWebFragmentsRelativeDuplicate() throws MalformedURLException {
+        WebXml withDuplicate = new WebXml();
+        withDuplicate.setURL(new URL("https://example.com/original.jar"));
+        withDuplicate.addDuplicate("https://example.com/duplicate.jar");
+
+        Map<String,WebXml> fragmentsWithDuplicate = new LinkedHashMap<>();
+        fragmentsWithDuplicate.put("has-duplicate", withDuplicate);
+
+        WebXml.orderWebFragments(app, fragmentsWithDuplicate, null);
+    }
+
+    @SuppressWarnings("deprecation")
+    @Test(expected=IllegalArgumentException.class)
+    public void testOrderWebFragmentsRelativeDuplicateDeprecated() throws MalformedURLException {
+        WebXml withDuplicate = new WebXml();
+        withDuplicate.setURL(new URL("https://example.com/original.jar"));
+        withDuplicate.setDuplicated(true);
+
+        Map<String,WebXml> fragmentsWithDuplicate = new LinkedHashMap<>();
+        fragmentsWithDuplicate.put("has-duplicate", withDuplicate);
+
+        WebXml.orderWebFragments(app, fragmentsWithDuplicate, null);
+    }
+
     private interface RelativeOrderingTestRunner {
         void init();
         void validate(String order);

webapps/docs/changelog.xml

@@ -152,6 +152,12 @@
         <bug>66233</bug>: Include an error message when sending a 400 response
         because a request has too many cookies. (markt)
       </fix>
+      <fix>
+        When web application deployment fails due to JARs with duplicate
+        fragment names, improve the error message by listing the JARs that
+        contain the duplicates. Based on pull request <pr>535</pr> by Mads
+        Rolsdorph. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">