fix: UriTemplate expansion reserved ("+") and fragment("#") should not encode already percent encoded parts (#2108)

* Added new escaper for already percent encoded inputs

* Added the new escaper to the CharEscapers utility class

* Fixed the inconsistency with rfc6570#section-3.2.1

* Fix linter error

* Update documentation to clarify support for URI Template levels

* Change PercentEncodedEscaper class to package-private

---------

Co-authored-by: Diego Marquez <[email protected]>

Author: alexander-asenov

google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java

@@ -29,8 +29,8 @@
 /**
  * Expands URI Templates.
  *
- * <p>This Class supports Level 1 templates and all Level 4 composite templates as described in: <a
- * href="http://tools.ietf.org/html/rfc6570">RFC 6570</a>.
+ * <p>This class supports URI Template Level 1, partial support for Levels 2 and 3, and Level 4
+ * composite templates as described in: <a href="http://tools.ietf.org/html/rfc6570">RFC 6570</a>.
  *
  * <p>Specifically, for the variables: var := "value" list := ["red", "green", "blue"] keys :=
  * [("semi", ";"),("dot", "."),("comma", ",")]
@@ -159,7 +159,7 @@ private String getEncodedValue(String value) {
       String encodedValue;
       if (reservedExpansion) {
         // Reserved expansion allows percent-encoded triplets and characters in the reserved set.
-        encodedValue = CharEscapers.escapeUriPathWithoutReserved(value);
+        encodedValue = CharEscapers.escapeUriPathWithoutReservedAndPercentEncoded(value);
       } else {
         encodedValue = CharEscapers.escapeUriConformant(value);
       }

google-http-client/src/main/java/com/google/api/client/util/escape/CharEscapers.java

@@ -44,6 +44,9 @@ public final class CharEscapers {
   private static final Escaper URI_QUERY_STRING_ESCAPER =
       new PercentEscaper(PercentEscaper.SAFEQUERYSTRINGCHARS_URLENCODER);
 
+  private static final Escaper URI_RESERVED_AND_PERCENT_ENCODED_ESCAPER =
+      new PercentEncodedEscaper(URI_RESERVED_ESCAPER);
+
   /**
    * Escapes the string value so it can be safely included in application/x-www-form-urlencoded
    * data. This is not appropriate for generic URI escaping. In particular it encodes the space
@@ -184,6 +187,15 @@ public static String escapeUriPathWithoutReserved(String value) {
     return URI_RESERVED_ESCAPER.escape(value);
   }
 
+  /**
+   * Escapes a URI path but retains all reserved and percent-encoded characters. That is the same as
+   * {@link #escapeUriPathWithoutReserved(String)} except that it also escapes percent encoded
+   * parts.
+   */
+  public static String escapeUriPathWithoutReservedAndPercentEncoded(String value) {
+    return URI_RESERVED_AND_PERCENT_ENCODED_ESCAPER.escape(value);
+  }
+
   /**
    * Escapes the string value so it can be safely included in URI user info part. For details on
    * escaping URIs, see <a href="http://tools.ietf.org/html/rfc3986#section-2.4">RFC 3986 - section

google-http-client/src/main/java/com/google/api/client/util/escape/PercentEncodedEscaper.java

@@ -0,0 +1,57 @@
+package com.google.api.client.util.escape;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * An {@link Escaper} implementation that preserves percent-encoded sequences in the input string.
+ *
+ * <p>This escaper applies the provided {@link Escaper} to all parts of the input string except for
+ * valid percent-encoded sequences (e.g., <code>%20</code>), which are left unchanged.
+ */
+final class PercentEncodedEscaper extends Escaper {
+
+  /** Pattern to match valid percent-encoded sequences (e.g., %20). */
+  static final Pattern PCT_ENCODE_PATTERN = Pattern.compile("%[0-9A-Fa-f]{2}");
+
+  private final Escaper escaper;
+
+  public PercentEncodedEscaper(Escaper escaper) {
+    if (escaper == null) {
+      throw new NullPointerException("Escaper cannot be null");
+    }
+    this.escaper = escaper;
+  }
+
+  /**
+   * Escapes the input string using the provided {@link Escaper}, preserving valid percent-encoded
+   * sequences.
+   *
+   * @param string the input string to escape
+   * @return the escaped string with percent-encoded sequences left unchanged
+   */
+  @Override
+  public String escape(String string) {
+    if (string == null || string.isEmpty()) {
+      return string;
+    }
+
+    Matcher matcher = PCT_ENCODE_PATTERN.matcher(string);
+    StringBuilder sb = new StringBuilder();
+
+    int lastEnd = 0;
+    while (matcher.find()) {
+      sb.append(escaper.escape(string.substring(lastEnd, matcher.start())));
+
+      sb.append(string.substring(matcher.start(), matcher.end()));
+
+      lastEnd = matcher.end();
+    }
+
+    if (lastEnd < string.length()) {
+      sb.append(escaper.escape(string.substring(lastEnd)));
+    }
+
+    return sb.toString();
+  }
+}

google-http-client/src/test/java/com/google/api/client/http/UriTemplateTest.java

@@ -380,4 +380,57 @@ public void testExpandTemplates_reservedExpansion_mustNotEscapeUnreservedCharSet
         unReservedSet,
         UriTemplate.expand("{+var}", requestMap, false));
   }
+
+  @Test
+  // These tests are from the uri-template test suite
+  // https://github.com/uri-templates/uritemplate-test/blob/master/extended-tests.json
+  public void testExpandTemplates_reservedExpansion_alreadyEncodedInput() {
+    Map<String, Object> variables = Maps.newLinkedHashMap();
+    variables.put("id", "admin%2F");
+    assertEquals("admin%252F", UriTemplate.expand("{id}", variables, false));
+    assertEquals("admin%2F", UriTemplate.expand("{+id}", variables, false));
+    assertEquals("#admin%2F", UriTemplate.expand("{#id}", variables, false));
+  }
+
+  @Test
+  // These tests are from the uri-template test suite
+  // https://github.com/uri-templates/uritemplate-test/blob/master/extended-tests.json
+  public void testExpandTemplates_reservedExpansion_notEncodedInput() {
+    Map<String, Object> variables = Maps.newLinkedHashMap();
+    variables.put("not_pct", "%foo");
+    assertEquals("%25foo", UriTemplate.expand("{not_pct}", variables, false));
+    assertEquals("%25foo", UriTemplate.expand("{+not_pct}", variables, false));
+    assertEquals("#%25foo", UriTemplate.expand("{#not_pct}", variables, false));
+  }
+
+  @Test
+  // These tests are from the uri-template test suite
+  // https://github.com/uri-templates/uritemplate-test/blob/master/extended-tests.json
+  public void testExpandTemplates_reservedExpansion_listExpansionWithMixedEncodedInput() {
+    Map<String, Object> variables = Maps.newLinkedHashMap();
+    variables.put("list", Arrays.asList("red%25", "%2Fgreen", "blue "));
+    assertEquals("red%2525,%252Fgreen,blue%20", UriTemplate.expand("{list}", variables, false));
+    assertEquals("red%25,%2Fgreen,blue%20", UriTemplate.expand("{+list}", variables, false));
+    assertEquals("#red%25,%2Fgreen,blue%20", UriTemplate.expand("{#list}", variables, false));
+  }
+
+  @Test
+  // These tests are from the uri-template test suite
+  // https://github.com/uri-templates/uritemplate-test/blob/master/extended-tests.json with an
+  // additional map entry
+  public void testExpandTemplates_reservedExpansion_mapWithMixedEncodedInput() {
+    Map<String, Object> variables = Maps.newLinkedHashMap();
+    Map<String, String> keys = Maps.newLinkedHashMap();
+    keys.put("key1", "val1%2F");
+    keys.put("key2", "val2%2F");
+    keys.put("key3", "val ");
+    variables.put("keys", keys);
+    assertEquals(
+        "key1,val1%252F,key2,val2%252F,key3,val%20",
+        UriTemplate.expand("{keys}", variables, false));
+    assertEquals(
+        "key1,val1%2F,key2,val2%2F,key3,val%20", UriTemplate.expand("{+keys}", variables, false));
+    assertEquals(
+        "#key1,val1%2F,key2,val2%2F,key3,val%20", UriTemplate.expand("{#keys}", variables, false));
+  }
 }

google-http-client/src/test/java/com/google/api/client/util/escape/PercentEncodedEscaperTest.java

@@ -0,0 +1,32 @@
+package com.google.api.client.util.escape;
+
+import junit.framework.TestCase;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class PercentEncodedEscaperTest extends TestCase {
+  @Test
+  public void testEscape() {
+    PercentEncodedEscaper escaper =
+        new PercentEncodedEscaper(
+            new PercentEscaper(PercentEscaper.SAFE_PLUS_RESERVED_CHARS_URLENCODER));
+    String input = "Hello%20World+/?#[]";
+
+    String actual = escaper.escape(input);
+    assertEquals(input, actual); // No change expected since it's already percent-encoded
+  }
+
+  @Test
+  public void testEscapeEncode() {
+    PercentEncodedEscaper escaper =
+        new PercentEncodedEscaper(
+            new PercentEscaper(PercentEscaper.SAFE_PLUS_RESERVED_CHARS_URLENCODER));
+    String input = "Hello World%";
+    String expected = "Hello%20World%25";
+
+    String actual = escaper.escape(input);
+    assertEquals(expected, actual);
+  }
+}