fix: allow non-lowercase qualifier keys (#169)

dwalluck · web-flow · commit b2f26f8c23dd · 2025-03-11T05:32:54.000-04:00
diff --git a/src/main/java/com/github/packageurl/PackageURL.java b/src/main/java/com/github/packageurl/PackageURL.java
@@ -31,6 +31,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.TreeMap;
+import java.util.function.IntPredicate;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
@@ -98,7 +99,7 @@ public PackageURL(final String type, final String namespace, final String name,
         this.namespace = validateNamespace(namespace);
         this.name = validateName(name);
         this.version = validateVersion(version);
-        this.qualifiers = validateQualifiers(qualifiers);
+        this.qualifiers = parseQualifiers(qualifiers);
         this.subpath = validatePath(subpath, true);
         verifyTypeConstraints(this.type, this.namespace, this.name);
     }
@@ -223,7 +224,7 @@ public String getVersion() {
      * @since 1.0.0
      */
     public Map<String, String> getQualifiers() {
-        return (qualifiers != null)? Collections.unmodifiableMap(qualifiers) : null;
+        return (qualifiers != null) ? Collections.unmodifiableMap(qualifiers) : null;
     }
 
     /**
@@ -247,18 +248,31 @@ private String validateType(final String value) throws MalformedPackageURLExcept
             throw new MalformedPackageURLException("The PackageURL type cannot be null or empty");
         }
 
-        if (isDigit(value.charAt(0))) {
-            throw new MalformedPackageURLException("The PackageURL type cannot start with a number");
-        }
+        validateChars(value, PackageURL::isValidCharForType, "type");
+
+        return value;
+    }
+
+    private static boolean isValidCharForType(int c) {
+        return (isAlphaNumeric(c) || c == '.' || c == '+' || c == '-');
+    }
 
-        if (!value.chars().allMatch(c -> (c == '.' || c == '+' || c == '-'
-                || isUpperCase(c)
-                || isLowerCase(c)
-                || isDigit(c)))) {
-            throw new MalformedPackageURLException("The PackageURL type contains invalid characters");
+    private static boolean isValidCharForKey(int c) {
+        return (isAlphaNumeric(c) || c == '.' || c == '_' || c == '-');
+    }
+
+    private static void validateChars(String value, IntPredicate predicate, String component) throws MalformedPackageURLException {
+        char firstChar = value.charAt(0);
+
+        if (isDigit(firstChar)) {
+            throw new MalformedPackageURLException("The PackageURL " + component + " cannot start with a number: " + firstChar);
         }
 
-        return value;
+        String invalidChars = value.chars().filter(predicate.negate()).mapToObj(c -> String.valueOf((char) c)).collect(Collectors.joining(", "));
+
+        if (!invalidChars.isEmpty()) {
+            throw new MalformedPackageURLException("The PackageURL " + component + " '" + value + "' contains invalid characters: " + invalidChars);
+        }
     }
 
     private String validateNamespace(final String value) throws MalformedPackageURLException {
@@ -319,7 +333,7 @@ private String validateVersion(final String value) {
     }
 
     private Map<String, String> validateQualifiers(final Map<String, String> values) throws MalformedPackageURLException {
-        if (values == null) {
+        if (values == null || values.isEmpty()) {
             return null;
         }
         for (Map.Entry<String, String> entry : values.entrySet()) {
@@ -337,10 +351,7 @@ private void validateKey(final String value) throws MalformedPackageURLException
             throw new MalformedPackageURLException("Qualifier key is invalid: " + value);
         }
 
-        if (isDigit(value.charAt(0))
-                || !value.chars().allMatch(c -> isLowerCase(c) || (isDigit(c)) || c == '.' || c == '-' || c == '_')) {
-            throw new MalformedPackageURLException("Qualifier key is invalid: " + value);
-        }
+        validateChars(value, PackageURL::isValidCharForKey, "qualifier key");
     }
 
     private String validatePath(final String value, final boolean isSubpath) throws MalformedPackageURLException {
@@ -463,7 +474,7 @@ private static String uriEncode(String source, Charset charset) {
     }
 
     private static boolean isUnreserved(int c) {
-        return (isAlpha(c) || isDigit(c) || '-' == c || '.' == c || '_' == c || '~' == c);
+        return (isValidCharForKey(c) || c == '~');
     }
 
     private static boolean isAlpha(int c) {
@@ -474,6 +485,10 @@ private static boolean isDigit(int c) {
         return (c >= '0' && c <= '9');
     }
 
+    private static boolean isAlphaNumeric(int c) {
+        return (isDigit(c) || isAlpha(c));
+    }
+
     private static boolean isUpperCase(int c) {
         return (c >= 'A' && c <= 'Z');
     }
@@ -656,6 +671,23 @@ private void verifyTypeConstraints(String type, String namespace, String name) t
         }
     }
 
+    private Map<String, String> parseQualifiers(final Map<String, String> qualifiers) throws MalformedPackageURLException {
+        if (qualifiers == null || qualifiers.isEmpty()) {
+            return null;
+        }
+
+        try {
+            final TreeMap<String, String> results = qualifiers.entrySet().stream()
+                    .filter(entry -> entry.getValue() != null && !entry.getValue().isEmpty())
+                    .collect(TreeMap::new,
+                            (map, value) -> map.put(toLowerCase(value.getKey()), value.getValue()),
+                            TreeMap::putAll);
+            return validateQualifiers(results);
+        } catch (ValidationException ex) {
+            throw new MalformedPackageURLException(ex.getMessage());
+        }
+    }
+
     @SuppressWarnings("StringSplitter")//reason: surprising behavior is okay in this case
     private Map<String, String> parseQualifiers(final String encodedString) throws MalformedPackageURLException {
         try {
diff --git a/src/test/java/com/github/packageurl/PackageURLBuilderTest.java b/src/test/java/com/github/packageurl/PackageURLBuilderTest.java
@@ -95,26 +95,22 @@ public void testPackageURLBuilder() throws MalformedPackageURLException {
 
     @Test
     public void testPackageURLBuilderException1() throws MalformedPackageURLException {
-        exception.expect(MalformedPackageURLException.class);
-        exception.expectMessage("contains a qualifier key with an empty or null");
         PackageURL purl = PackageURLBuilder.aPackageURL()
                 .withType("type")
                 .withName("name")
                 .withQualifier("key","")
                 .build();
-        Assert.fail("Build should fail due to invalid qualifier (empty value)");
+        assertNull(purl.getQualifiers());
     }
 
     @Test
     public void testPackageURLBuilderException1Null() throws MalformedPackageURLException {
-        exception.expect(MalformedPackageURLException.class);
-        exception.expectMessage("contains a qualifier key with an empty or null");
-        PackageURLBuilder.aPackageURL()
+        PackageURL purl = PackageURLBuilder.aPackageURL()
                 .withType("type")
                 .withName("name")
                 .withQualifier("key",null)
                 .build();
-        Assert.fail("Build should fail due to invalid qualifier (null value)");
+        assertNull(purl.getQualifiers());
     }
 
     @Test
@@ -216,4 +212,4 @@ private void assertBuilderMatch(PackageURL expected, PackageURLBuilder actual) t
 
     }
 
-}
+}
diff --git a/src/test/java/com/github/packageurl/PackageURLTest.java b/src/test/java/com/github/packageurl/PackageURLTest.java
@@ -23,11 +23,13 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.Locale;
 import java.util.TreeMap;
 
 import org.apache.commons.io.IOUtils;
 import org.json.JSONArray;
 import org.json.JSONObject;
+import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Rule;
@@ -42,17 +44,25 @@
  * @author Steve Springett
  */
 public class PackageURLTest {
-
     @Rule
     public ExpectedException exception = ExpectedException.none();
 
     private static JSONArray json = new JSONArray();
 
+    private static Locale defaultLocale;
+
     @BeforeClass
     public static void setup() throws IOException {
         InputStream is = PackageURLTest.class.getResourceAsStream("/test-suite-data.json");
         String jsonTxt = IOUtils.toString(is, "UTF-8");
         json = new JSONArray(jsonTxt);
+        defaultLocale = Locale.getDefault();
+        Locale.setDefault(new Locale("tr"));
+    }
+
+    @AfterClass
+    public static void resetLocale() {
+        Locale.setDefault(defaultLocale);
     }
 
     @Test
@@ -266,6 +276,38 @@ public void testConstructorWithDuplicateQualifiers() throws MalformedPackageURLE
         Assert.fail("constructor with url with duplicate qualifiers should have thrown an error and this line should not be reached");
     }
 
+    @Test
+    public void testConstructorDuplicateQualifiersMixedCase() throws MalformedPackageURLException {
+        exception.expect(MalformedPackageURLException.class);
+
+        PackageURL purl = new PackageURL("pkg://generic/name?key=one&KEY=two");
+        Assert.fail("constructor with url with duplicate qualifiers should have thrown an error and this line should not be reached");
+    }
+
+    @Test
+    public void testConstructorWithUppercaseKey() throws MalformedPackageURLException {
+        PackageURL purl = new PackageURL("pkg://generic/name?KEY=one");
+        Assert.assertNotNull(purl.getQualifiers());
+        Assert.assertEquals("one", purl.getQualifiers().get("key"));
+        TreeMap<String, String> qualifiers = new TreeMap<>();
+        qualifiers.put("key", "one");
+        PackageURL purl2 = new PackageURL("generic", null, "name", null, qualifiers, null);
+        Assert.assertEquals(purl, purl2);
+    }
+
+    @Test
+    public void testConstructorWithEmptyKey() throws MalformedPackageURLException {
+        PackageURL purl = new PackageURL("pkg://generic/name?KEY");
+        Assert.assertNull(purl.getQualifiers());
+        TreeMap<String, String> qualifiers = new TreeMap<>();
+        qualifiers.put("KEY", null);
+        PackageURL purl2 = new PackageURL("generic", null, "name", null, qualifiers, null);
+        Assert.assertEquals(purl, purl2);
+        qualifiers.put("KEY", "");
+        PackageURL purl3 = new PackageURL("generic", null, "name", null, qualifiers, null);
+        Assert.assertEquals(purl2, purl3);
+    }
+
     @Test
     public void testStandardTypes() {
         exception = ExpectedException.none();
