Fix URL encoding and decoding

The methods `uriEncode` and `uriDecode` did not properly handle
percent-encoding. In particular, `uriEncode` didn't properly output two
uppercase hex digits and `urlDecode` did not properly handle non-ASCII
characters.

Aditionally, if no percent-encoding was performed, these methods will
now return the original string.

Fixes package-url#150
Closes package-url#153
Fixes package-url#154

Author: dwalluck

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

@@ -21,10 +21,11 @@
  */
 package com.github.packageurl;
 
+import java.io.ByteArrayOutputStream;
 import java.io.Serializable;
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.nio.charset.Charset;
+import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.Collections;
@@ -34,6 +35,7 @@
 import java.util.function.IntPredicate;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 
 /**
  * <p>Package-URL (aka purl) is a "mostly universal" URL to describe a package. A purl is a URL composed of seven components:</p>
@@ -459,39 +461,14 @@ private String canonicalize(boolean coordinatesOnly) {
         return purl.toString();
     }
 
-    /**
-     * Encodes the input in conformance with RFC 3986.
-     *
-     * @param input the String to encode
-     * @return an encoded String
-     */
-    private String percentEncode(final String input) {
-        return uriEncode(input, StandardCharsets.UTF_8);
-    }
-
-    private static String uriEncode(String source, Charset charset) {
-        if (source == null || source.isEmpty()) {
-            return source;
-        }
-
-        StringBuilder builder = new StringBuilder();
-        for (byte b : source.getBytes(charset)) {
-            if (isUnreserved(b)) {
-                builder.append((char) b);
-            }
-            else {
-                // Substitution: A '%' followed by the hexadecimal representation of the ASCII value of the replaced character
-                builder.append('%');
-                builder.append(Integer.toHexString(b).toUpperCase());
-            }
-        }
-        return builder.toString();
-    }
-
     private static boolean isUnreserved(int c) {
         return (isValidCharForKey(c) || c == '~');
     }
 
+    private static boolean shouldEncode(int c) {
+        return !isUnreserved(c);
+    }
+
     private static boolean isAlpha(int c) {
         return (isLowerCase(c) || isUpperCase(c));
     }
@@ -547,42 +524,90 @@ private static String toLowerCase(String s) {
         return new String(chars);
     }
 
-    /**
-     * Optionally decodes a String, if it's encoded. If String is not encoded,
-     * method will return the original input value.
-     *
-     * @param input the value String to decode
-     * @return a decoded String
-     */
-    private String percentDecode(final String input) {
-        if (input == null) {
-            return null;
-        }
-        final String decoded = uriDecode(input);
-        if (!decoded.equals(input)) {
-            return decoded;
+    private static String percentDecode(final String source) {
+        if (source == null || source.isEmpty()) {
+            return source;
         }
-        return input;
-    }
 
-    public static String uriDecode(String source) {
-        if (source == null) {
+        byte[] bytes = source.getBytes(StandardCharsets.UTF_8);
+        int percentCharCount = getPercentCharCount(bytes);
+
+        if (percentCharCount == 0) {
             return source;
         }
-        int length = source.length();
-        StringBuilder builder = new StringBuilder();
+
+        int length = bytes.length;
+        int capacity = (length + percentCharCount) - (percentCharCount * 3);
+
+        if (capacity <= 0) {
+            throw new ValidationException("Invalid encoding in '" + source + "'");
+        }
+
+        ByteBuffer buffer = ByteBuffer.allocate(capacity);
+
         for (int i = 0; i < length; i++) {
-            if (source.charAt(i) == '%') {
-                String str = source.substring(i + 1, i + 3);
-                char c = (char) Integer.parseInt(str, 16);
-                builder.append(c);
-                i += 2;
+            int b = bytes[i];
+
+            if (b == '%') {
+                if (i + 2 < length) {
+                    throw new ValidationException("Invalid encoding in '" + source + "' at position " + i);
+                }
+
+                int b1 = Character.digit(bytes[++i], 16);
+                int b2 = Character.digit(bytes[++i], 16);
+                buffer.put((byte) ((b1 << 4) + b2));
+            } else {
+                buffer.put((byte) b);
             }
-            else {
-                builder.append(source.charAt(i));
+        }
+
+        return new String(buffer.array(),StandardCharsets.UTF_8);
+    }
+
+    @Deprecated
+    public String uriDecode(final String source) {
+        return percentDecode(source);
+    }
+
+    private static int getUnsafeCharCount(final byte[] bytes) {
+        return (int)  IntStream.range(0, bytes.length).map(i -> bytes[i]).filter(PackageURL::shouldEncode).count();
+    }
+
+    private static boolean isPercent(int c) {
+        return (c == '%');
+    }
+
+    private static int getPercentCharCount(final byte[] bytes) {
+        return (int)  IntStream.range(0, bytes.length).map(i -> bytes[i]).filter(PackageURL::isPercent).count();
+    }
+
+    private static String percentEncode(final String source) {
+        if (source == null || source.isEmpty()) {
+            return source;
+        }
+
+        byte[] bytes = source.getBytes(StandardCharsets.UTF_8);
+        int unsafeCharCount = getUnsafeCharCount(bytes);
+
+        if (unsafeCharCount == 0) {
+            return source;
+        }
+
+        int length = bytes.length;
+        int capacity = (length - unsafeCharCount) + (3 * unsafeCharCount);
+        ByteBuffer bb = ByteBuffer.allocate(capacity);
+
+        for (byte b : bytes) {
+            if (shouldEncode(b)) {
+                bb.put((byte) '%');
+                bb.put((byte) Character.toUpperCase(Character.forDigit((b >> 4) & 0xF, 16)));
+                bb.put((byte) Character.toUpperCase(Character.forDigit(b & 0xF, 16)));
+            } else {
+                bb.put(b);
             }
         }
-        return builder.toString();
+
+        return new String(bb.array(), StandardCharsets.UTF_8);
     }
 
     /**
@@ -652,16 +677,16 @@ private void parse(final String purl) throws MalformedPackageURLException {
             // version is optional - check for existence
             index = remainder.lastIndexOf('@');
             if (index >= start) {
-                this.version = validateVersion(percentDecode(remainder.substring(index + 1)));
+                this.version = validateVersion(uriDecode(remainder.substring(index + 1)));
                 remainder = remainder.substring(0, index);
             }
 
             // The 'remainder' should now consist of an optional namespace and the name
             index = remainder.lastIndexOf('/');
             if (index <= start) {
-                this.name = validateName(percentDecode(remainder.substring(start)));
+                this.name = validateName(uriDecode(remainder.substring(start)));
             } else {
-                this.name = validateName(percentDecode(remainder.substring(index + 1)));
+                this.name = validateName(uriDecode(remainder.substring(index + 1)));
                 remainder = remainder.substring(0, index);
                 this.namespace = validateNamespace(parsePath(remainder.substring(start), false));
             }
@@ -712,7 +737,7 @@ private Map<String, String> parseQualifiers(final String encodedString) throws M
                                 final String[] entry = value.split("=", 2);
                                 if (entry.length == 2 && !entry[1].isEmpty()) {
                                     String key = toLowerCase(entry[0]);
-                                    if (map.put(key, percentDecode(entry[1])) != null) {
+                                    if (map.put(key, uriDecode(entry[1])) != null) {
                                         throw new ValidationException("Duplicate package qualifier encountered. More then one value was specified for " + key);
                                     }
                                 }
@@ -731,7 +756,7 @@ private String[] parsePath(final String value, final boolean isSubpath) {
         }
         return PATH_SPLITTER.splitAsStream(value)
                 .filter(segment -> !segment.isEmpty() && !(isSubpath && (".".equals(segment) || "..".equals(segment))))
-                .map(segment -> percentDecode(segment))
+                .map(segment -> uriDecode(segment))
                 .toArray(String[]::new);
     }
 

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

@@ -70,6 +70,30 @@ public static void resetLocale() {
         Locale.setDefault(defaultLocale);
     }
 
+    @Test
+    public void testEncoding1() throws MalformedPackageURLException {
+        PackageURL purl = new PackageURL("maven", "com.google.summit", "summit-ast", "2.2.0\n", null, null);
+        Assert.assertEquals("pkg:maven/com.google.summit/summit-ast@2.2.0%0A", purl.toString());
+    }
+
+    @Test
+    public void testEncoding2() throws MalformedPackageURLException {
+        PackageURL purl = new PackageURL("pkg:nuget/%D0%9Cicros%D0%BEft.%D0%95ntit%D1%83Fram%D0%B5work%D0%A1%D0%BEr%D0%B5");
+        Assert.assertEquals("Мicrosоft.ЕntitуFramеworkСоrе", purl.getName());
+        Assert.assertEquals("pkg:nuget/%D0%9Cicros%D0%BEft.%D0%95ntit%D1%83Fram%D0%B5work%D0%A1%D0%BEr%D0%B5", purl.toString());
+    }
+
+    @SuppressWarnings("deprecation")
+    @Test
+    public void testInvalidPercentEncoding() throws MalformedPackageURLException {
+        Assert.assertThrows(MalformedPackageURLException.class, () -> new PackageURL("pkg:maven/com.google.summit/summit-ast@2.2.0%"));
+        Assert.assertThrows(MalformedPackageURLException.class, () -> new PackageURL("pkg:maven/com.google.summit/summit-ast@2.2.0%0"));
+        PackageURL packageURL = new PackageURL("pkg:maven/com.google.summit/summit-ast@2.2.0");
+        Assert.assertThrows(ValidationException.class, () -> packageURL.uriDecode("%"));
+        Assert.assertThrows(ValidationException.class, () -> packageURL.uriDecode("%0"));
+        Assert.assertThrows(ValidationException.class, () -> packageURL.uriDecode("abc%0a0%"));
+    }
+
     @Test
     public void testConstructorParsing() throws Exception {
         exception = ExpectedException.none();