Skip to content

Commit 1494768

Browse files
dwalluckdwalluck
committed
Allow non-lowercase qualifier keys
1 parent dd8309b commit 1494768

File tree

3 files changed

+93
-23
lines changed

3 files changed

+93
-23
lines changed

‎src/main/java/com/github/packageurl/PackageURL.java‎

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public PackageURL(final String type, final String namespace, final String name,
9898
this.namespace = validateNamespace(namespace);
9999
this.name = validateName(name);
100100
this.version = validateVersion(version);
101-
this.qualifiers = validateQualifiers(qualifiers);
101+
this.qualifiers = parseQualifiers(qualifiers);
102102
this.subpath = validatePath(subpath, true);
103103
verifyTypeConstraints(this.type, this.namespace, this.name);
104104
}
@@ -223,7 +223,7 @@ public String getVersion() {
223223
* @since 1.0.0
224224
*/
225225
public Map<String, String> getQualifiers() {
226-
return (qualifiers != null)? Collections.unmodifiableMap(qualifiers) : null;
226+
return (qualifiers != null) ? Collections.unmodifiableMap(qualifiers) : null;
227227
}
228228

229229
/**
@@ -247,15 +247,17 @@ private String validateType(final String value) throws MalformedPackageURLExcept
247247
throw new MalformedPackageURLException("The PackageURL type cannot be null or empty");
248248
}
249249

250-
if (isDigit(value.charAt(0))) {
251-
throw new MalformedPackageURLException("The PackageURL type cannot start with a number");
250+
char firstChar = value.charAt(0);
251+
252+
if (isDigit(firstChar)) {
253+
throw new MalformedPackageURLException("The PackageURL type cannot start with a number: " + firstChar);
252254
}
253255

254-
if (!value.chars().allMatch(c -> (c == '.' || c == '+' || c == '-'
255-
|| isUpperCase(c)
256-
|| isLowerCase(c)
257-
|| isDigit(c)))) {
258-
throw new MalformedPackageURLException("The PackageURL type contains invalid characters");
256+
String invalidChars = value.chars().filter(c -> !(c == '.' || c == '+' || c == '-'
257+
|| isAlphaNumeric(c))).mapToObj(c -> String.valueOf((char) c)).collect(Collectors.joining(", "));
258+
259+
if (!invalidChars.isEmpty()) {
260+
throw new MalformedPackageURLException("The PackageURL type " + "'" + value + "' contains invalid characters: " + invalidChars);
259261
}
260262

261263
return value;
@@ -319,7 +321,7 @@ private String validateVersion(final String value) {
319321
}
320322

321323
private Map<String, String> validateQualifiers(final Map<String, String> values) throws MalformedPackageURLException {
322-
if (values == null) {
324+
if (values == null || values.isEmpty()) {
323325
return null;
324326
}
325327
for (Map.Entry<String, String> entry : values.entrySet()) {
@@ -337,9 +339,17 @@ private void validateKey(final String value) throws MalformedPackageURLException
337339
throw new MalformedPackageURLException("Qualifier key is invalid: " + value);
338340
}
339341

340-
if (isDigit(value.charAt(0))
341-
|| !value.chars().allMatch(c -> isLowerCase(c) || (isDigit(c)) || c == '.' || c == '-' || c == '_')) {
342-
throw new MalformedPackageURLException("Qualifier key is invalid: " + value);
342+
char firstChar = value.charAt(0);
343+
344+
if (isDigit(firstChar)) {
345+
throw new MalformedPackageURLException("The PackageURL type cannot start with a number: " + firstChar);
346+
}
347+
348+
String invalidChars = value.chars().filter(c -> !(c == '.' || c == '-' || c == '_'
349+
|| isAlphaNumeric(c))).mapToObj(c -> String.valueOf((char) c)).collect(Collectors.joining(", "));
350+
351+
if (!invalidChars.isEmpty()) {
352+
throw new MalformedPackageURLException("The PackageURL qualifier key " + "'" + value + "' contains invalid characters: " + invalidChars);
343353
}
344354
}
345355

@@ -463,7 +473,7 @@ private static String uriEncode(String source, Charset charset) {
463473
}
464474

465475
private static boolean isUnreserved(int c) {
466-
return (isAlpha(c) || isDigit(c) || '-' == c || '.' == c || '_' == c || '~' == c);
476+
return (isAlphaNumeric(c) || '-' == c || '.' == c || '_' == c || '~' == c);
467477
}
468478

469479
private static boolean isAlpha(int c) {
@@ -474,6 +484,10 @@ private static boolean isDigit(int c) {
474484
return (c >= '0' && c <= '9');
475485
}
476486

487+
private static boolean isAlphaNumeric(int c) {
488+
return (isDigit(c) || isAlpha(c));
489+
}
490+
477491
private static boolean isUpperCase(int c) {
478492
return (c >= 'A' && c <= 'Z');
479493
}
@@ -656,6 +670,24 @@ private void verifyTypeConstraints(String type, String namespace, String name) t
656670
}
657671
}
658672

673+
private Map<String, String> parseQualifiers(final Map<String, String> qualifiers) throws MalformedPackageURLException {
674+
if (qualifiers == null || qualifiers.isEmpty()) {
675+
return null;
676+
}
677+
678+
try {
679+
final TreeMap<String, String> results = qualifiers.entrySet().stream()
680+
.filter(entry -> entry.getValue() != null && !entry.getValue().isEmpty())
681+
.collect(TreeMap::new,
682+
(map, value) -> map.put(toLowerCase(value.getKey()), value.getValue()),
683+
TreeMap::putAll);
684+
return validateQualifiers(results);
685+
} catch (ValidationException ex) {
686+
throw new MalformedPackageURLException(ex.getMessage());
687+
}
688+
}
689+
690+
659691
@SuppressWarnings("StringSplitter")//reason: surprising behavior is okay in this case
660692
private Map<String, String> parseQualifiers(final String encodedString) throws MalformedPackageURLException {
661693
try {

‎src/test/java/com/github/packageurl/PackageURLBuilderTest.java‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -95,26 +95,22 @@ public void testPackageURLBuilder() throws MalformedPackageURLException {
9595

9696
@Test
9797
public void testPackageURLBuilderException1() throws MalformedPackageURLException {
98-
exception.expect(MalformedPackageURLException.class);
99-
exception.expectMessage("contains a qualifier key with an empty or null");
10098
PackageURL purl = PackageURLBuilder.aPackageURL()
10199
.withType("type")
102100
.withName("name")
103101
.withQualifier("key","")
104102
.build();
105-
Assert.fail("Build should fail due to invalid qualifier (empty value)");
103+
assertNull(purl.getQualifiers());
106104
}
107105

108106
@Test
109107
public void testPackageURLBuilderException1Null() throws MalformedPackageURLException {
110-
exception.expect(MalformedPackageURLException.class);
111-
exception.expectMessage("contains a qualifier key with an empty or null");
112-
PackageURLBuilder.aPackageURL()
108+
PackageURL purl = PackageURLBuilder.aPackageURL()
113109
.withType("type")
114110
.withName("name")
115111
.withQualifier("key",null)
116112
.build();
117-
Assert.fail("Build should fail due to invalid qualifier (null value)");
113+
assertNull(purl.getQualifiers());
118114
}
119115

120116
@Test
@@ -216,4 +212,4 @@ private void assertBuilderMatch(PackageURL expected, PackageURLBuilder actual) t
216212

217213
}
218214

219-
}
215+
}

‎src/test/java/com/github/packageurl/PackageURLTest.java‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,13 @@
2323

2424
import java.io.IOException;
2525
import java.io.InputStream;
26+
import java.util.Locale;
2627
import java.util.TreeMap;
2728

2829
import org.apache.commons.io.IOUtils;
2930
import org.json.JSONArray;
3031
import org.json.JSONObject;
32+
import org.junit.AfterClass;
3133
import org.junit.Assert;
3234
import org.junit.BeforeClass;
3335
import org.junit.Rule;
@@ -42,17 +44,25 @@
4244
* @author Steve Springett
4345
*/
4446
public class PackageURLTest {
45-
4647
@Rule
4748
public ExpectedException exception = ExpectedException.none();
4849

4950
private static JSONArray json = new JSONArray();
5051

52+
private static Locale defaultLocale;
53+
5154
@BeforeClass
5255
public static void setup() throws IOException {
5356
InputStream is = PackageURLTest.class.getResourceAsStream("/test-suite-data.json");
5457
String jsonTxt = IOUtils.toString(is, "UTF-8");
5558
json = new JSONArray(jsonTxt);
59+
defaultLocale = Locale.getDefault();
60+
Locale.setDefault(new Locale("tr"));
61+
}
62+
63+
@AfterClass
64+
public static void resetLocale() {
65+
Locale.setDefault(defaultLocale);
5666
}
5767

5868
@Test
@@ -266,6 +276,38 @@ public void testConstructorWithDuplicateQualifiers() throws MalformedPackageURLE
266276
Assert.fail("constructor with url with duplicate qualifiers should have thrown an error and this line should not be reached");
267277
}
268278

279+
@Test
280+
public void testConstructorDuplicateQualifiersMixedCase() throws MalformedPackageURLException {
281+
exception.expect(MalformedPackageURLException.class);
282+
283+
PackageURL purl = new PackageURL("pkg://generic/name?key=one&KEY=two");
284+
Assert.fail("constructor with url with duplicate qualifiers should have thrown an error and this line should not be reached");
285+
}
286+
287+
@Test
288+
public void testConstructorWithUppercaseKey() throws MalformedPackageURLException {
289+
PackageURL purl = new PackageURL("pkg://generic/name?KEY=one");
290+
Assert.assertNotNull(purl.getQualifiers());
291+
Assert.assertEquals("one", purl.getQualifiers().get("key"));
292+
TreeMap<String, String> qualifiers = new TreeMap<>();
293+
qualifiers.put("key", "one");
294+
PackageURL purl2 = new PackageURL("generic", null, "name", null, qualifiers, null);
295+
Assert.assertEquals(purl, purl2);
296+
}
297+
298+
@Test
299+
public void testConstructorWithEmptyKey() throws MalformedPackageURLException {
300+
PackageURL purl = new PackageURL("pkg://generic/name?KEY");
301+
Assert.assertNull(purl.getQualifiers());
302+
TreeMap<String, String> qualifiers = new TreeMap<>();
303+
qualifiers.put("KEY", null);
304+
PackageURL purl2 = new PackageURL("generic", null, "name", null, qualifiers, null);
305+
Assert.assertEquals(purl, purl2);
306+
qualifiers.put("KEY", "");
307+
PackageURL purl3 = new PackageURL("generic", null, "name", null, qualifiers, null);
308+
Assert.assertEquals(purl2, purl3);
309+
}
310+
269311
@Test
270312
public void testStandardTypes() {
271313
exception = ExpectedException.none();

0 commit comments

Comments
 (0)