Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
*/
public 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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
Loading