Preserve malformed percent sequences in URI decoding (4.x) (helidon-io#11910) (helidon-io#12029)

tomas-langer · web-flow · commit c5b0ca1dd1ef · 2026-06-02T18:01:52.000+02:00
* Preserve malformed URI percent sequences * Preserve malformed query percent escapes * Preserve adjacent malformed URI escapes * Cover adjacent incomplete URI escapes (cherry picked from commit 08de312)
diff --git a/common/uri/src/main/java/io/helidon/common/uri/UriEncoding.java b/common/uri/src/main/java/io/helidon/common/uri/UriEncoding.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2025 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2026 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -46,6 +46,7 @@ private UriEncoding() {
      * <p>
      * Percent characters {@code "%s"} found between brackets {@code "[]"} are not decoded to support IPv6 literal.
      * E.g. {@code http://[fe80::1%lo0]:8080}.
+     * Malformed percent-encoded sequences are preserved literally.
      * <p>
      * See <a href="https://tools.ietf.org/html/rfc6874#section-2">RFC 6874, section 2.</a>
      *
@@ -58,6 +59,7 @@ public static String decodeUri(String uriSegment) {
 
     /**
      * Decode a URI query.
+     * Malformed percent-encoded sequences are preserved literally.
      *
      * @param uriQuery URI query with percent encoding
      * @return decoded string
@@ -169,17 +171,34 @@ private static String decode(String string, boolean ignorePercentInBrackets) {
                 c = string.charAt(i);
                 continue;
             }
-            bb.clear();
-            while (true) {
-                bb.put(decode(string.charAt(++i), string.charAt(++i)));
-                if (++i >= len) {
+            if (!isPercentEncoded(string, i, len)) {
+                int malformedEnd = malformedPercentEnd(string, i, len);
+                while (i < malformedEnd) {
+                    c = string.charAt(i);
+                    if (c == '[') {
+                        betweenBrackets = true;
+                    } else if (betweenBrackets && c == ']') {
+                        betweenBrackets = false;
+                    }
+                    sb.append(c);
+                    i++;
+                }
+                if (i >= len) {
                     break;
                 }
                 c = string.charAt(i);
-                if (c != '%') {
+                continue;
+            }
+
+            bb.clear();
+            do {
+                bb.put(decode(string.charAt(i + 1), string.charAt(i + 2)));
+                i += 3;
+                if (i >= len) {
                     break;
                 }
-            }
+                c = string.charAt(i);
+            } while (c == '%' && isPercentEncoded(string, i, len));
             bb.flip();
 
             CharBuffer cb = StandardCharsets.UTF_8.decode(bb);
@@ -207,6 +226,22 @@ private static int decode(char c) {
         return -1;
     }
 
+    private static boolean isPercentEncoded(String string, int index, int len) {
+        return index + 2 < len
+                && isHexCharacter(string.charAt(index + 1))
+                && isHexCharacter(string.charAt(index + 2));
+    }
+
+    private static int malformedPercentEnd(String string, int index, int len) {
+        int end = Math.min(index + 3, len);
+        for (int i = index + 1; i < end; i++) {
+            if (string.charAt(i) == '%') {
+                return i;
+            }
+        }
+        return end;
+    }
+
     private static void appendUTF8EncodedCharacter(StringBuilder sb, int codePoint) {
         CharBuffer chars = CharBuffer.wrap(Character.toChars(codePoint));
         ByteBuffer bytes = StandardCharsets.UTF_8.encode(chars);
diff --git a/common/uri/src/main/java/io/helidon/common/uri/UriQuery.java b/common/uri/src/main/java/io/helidon/common/uri/UriQuery.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2024 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2026 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -32,6 +32,8 @@ public interface UriQuery extends Parameters {
     /**
      * Create a new HTTP query from the query string.
      * This method does not validate the raw query against specification.
+     * Malformed percent-encoded sequences are preserved literally when parameter names and values are decoded.
+     * Use {@link #create(String, boolean) create(query, true)} for strict query validation.
      *
      * @param query raw query string
      * @return HTTP query instance
diff --git a/common/uri/src/main/java/io/helidon/common/uri/UriQueryWriteable.java b/common/uri/src/main/java/io/helidon/common/uri/UriQueryWriteable.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2024 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2026 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -89,6 +89,9 @@ static UriQueryWriteable create() {
      * <p>
      * This documentation (and behavior) has been changed, as we cannot create a proper query from {@code decoded} values,
      *  as these may contain characters used to split the query.
+     * Malformed percent-encoded sequences are preserved literally when parameter names and values are decoded.
+     * Use {@link UriValidator#validateQuery(String)} for strict query validation before updating this instance.
+     *
      * @param queryString encoded query string to update this instance
      */
     void fromQueryString(String queryString);
diff --git a/common/uri/src/test/java/io/helidon/common/uri/UriEncodingTest.java b/common/uri/src/test/java/io/helidon/common/uri/UriEncodingTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2024 Oracle and/or its affiliates.
+ * Copyright (c) 2024, 2026 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,6 +20,7 @@
 import static io.helidon.common.uri.UriEncoding.decodeUri;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.CoreMatchers.is;
+import static org.junit.jupiter.api.Assertions.assertAll;
 
 class UriEncodingTest {
 
@@ -35,4 +36,26 @@ void testSpaceDecoding() {
     void testIPv6Literal() {
         assertThat(decodeUri("http://[fe80::1%lo0]:8080"), is("http://[fe80::1%lo0]:8080"));
     }
+
+    @Test
+    void malformedHexEscapesRemainLiteral() {
+        assertAll(
+                () -> assertThat(decodeUri("%2G"), is("%2G")),
+                () -> assertThat(decodeUri("%G2"), is("%G2")),
+                () -> assertThat(decodeUri("%+1"), is("%+1")),
+                () -> assertThat(decodeUri("%G%41"), is("%GA")),
+                () -> assertThat(decodeUri("%4%41"), is("%4A")),
+                () -> assertThat(decodeUri("%%41"), is("%A")),
+                () -> assertThat(decodeUri("%41%2G%42"), is("A%2GB")),
+                () -> assertThat(decodeUri("prefix%G2suffix"), is("prefix%G2suffix")));
+    }
+
+    @Test
+    void incompletePercentEscapesRemainLiteral() {
+        assertAll(
+                () -> assertThat(decodeUri("%"), is("%")),
+                () -> assertThat(decodeUri("%4"), is("%4")),
+                () -> assertThat(decodeUri("%41%"), is("A%")),
+                () -> assertThat(decodeUri("prefix%4suffix"), is("prefix%4suffix")));
+    }
 }
diff --git a/common/uri/src/test/java/io/helidon/common/uri/UriQueryTest.java b/common/uri/src/test/java/io/helidon/common/uri/UriQueryTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2024 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2026 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -27,6 +27,7 @@
 import static org.hamcrest.CoreMatchers.hasItems;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.core.Is.is;
+import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 class UriQueryTest {
@@ -63,6 +64,40 @@ void testEncodedOtherChars() {
         assertThat(uriQuery.get("a"), is("b&c=d"));
     }
 
+    @Test
+    void malformedHexEscapesRemainLiteral() {
+        assertAll(
+                () -> assertThat(UriQuery.create("value=%2G").get("value"), is("%2G")),
+                () -> assertThat(UriQuery.create("value=%G2").get("value"), is("%G2")),
+                () -> assertThat(UriQuery.create("value=%+1").get("value"), is("%+1")),
+                () -> assertThat(UriQuery.create("value=%G%41").get("value"), is("%GA")),
+                () -> assertThat(UriQuery.create("value=%4%41").get("value"), is("%4A")),
+                () -> assertThat(UriQuery.create("value=%%41").get("value"), is("%A")),
+                () -> assertThat(UriQuery.create("%2G=value").get("%2G"), is("value")),
+                () -> assertThat(UriQuery.create("%+1=value").get("%+1"), is("value")),
+                () -> assertThat(UriQuery.create("%G%41=value").get("%GA"), is("value")),
+                () -> assertThat(writeableQuery("value=%2G").get("value"), is("%2G")),
+                () -> assertThat(writeableQuery("value=%G2").get("value"), is("%G2")),
+                () -> assertThat(writeableQuery("value=%+1").get("value"), is("%+1")),
+                () -> assertThat(writeableQuery("value=%G%41").get("value"), is("%GA")),
+                () -> assertThat(writeableQuery("value=%4%41").get("value"), is("%4A")),
+                () -> assertThat(writeableQuery("value=%%41").get("value"), is("%A")),
+                () -> assertThat(writeableQuery("%G2=value").get("%G2"), is("value")),
+                () -> assertThat(writeableQuery("%+1=value").get("%+1"), is("value")),
+                () -> assertThat(writeableQuery("%G%41=value").get("%GA"), is("value")));
+    }
+
+    @Test
+    void incompletePercentEscapesRemainLiteral() {
+        assertAll(
+                () -> assertThat(UriQuery.create("value=%").get("value"), is("%")),
+                () -> assertThat(UriQuery.create("value=%4").get("value"), is("%4")),
+                () -> assertThat(UriQuery.create("%=value").get("%"), is("value")),
+                () -> assertThat(writeableQuery("value=%").get("value"), is("%")),
+                () -> assertThat(writeableQuery("value=%4").get("value"), is("%4")),
+                () -> assertThat(writeableQuery("%4=value").get("%4"), is("value")));
+    }
+
     @Test
     void testEmptyQueryString() {
         UriQuery uriQuery = UriQuery.create("");
@@ -103,4 +138,10 @@ void testFromQueryString() {
         assertThat(query.get("p4"), is("a b c"));
         assertThat(query.getRaw("p4"), is("a%20b%20c"));
     }
-}
+
+    private static UriQueryWriteable writeableQuery(String queryString) {
+        UriQueryWriteable query = UriQueryWriteable.create();
+        query.fromQueryString(queryString);
+        return query;
+    }
+}
