Do not encode '/' in path replacement when encoded=true.

square · Feb 4, 2016 · 01d6fb1 · 01d6fb1 · spacitron · Mar 22, 2016
1 parent 3c195a0
commit 01d6fb1
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 11 deletions.
diff --git a/retrofit/src/main/java/retrofit2/RequestBuilder.java b/retrofit/src/main/java/retrofit2/RequestBuilder.java
@@ -29,7 +29,7 @@
 final class RequestBuilder {
   private static final char[] HEX_DIGITS =
       { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' };
-  private static final String PATH_SEGMENT_ENCODE_SET = " \"<>^`{}|/\\?#";
+  private static final String PATH_SEGMENT_ALWAYS_ENCODE_SET = " \"<>^`{}|\\?#";
 
   private final String method;
 
@@ -85,20 +85,20 @@ void addPathParam(String name, String value, boolean encoded) {
       // The relative URL is cleared when the first query parameter is set.
       throw new AssertionError();
     }
-    relativeUrl = relativeUrl.replace("{" + name + "}", canonicalize(value, encoded));
+    relativeUrl = relativeUrl.replace("{" + name + "}", canonicalizeForPath(value, encoded));
   }
 
-  private static String canonicalize(String input, boolean alreadyEncoded) {
+  private static String canonicalizeForPath(String input, boolean alreadyEncoded) {
     int codePoint;
     for (int i = 0, limit = input.length(); i < limit; i += Character.charCount(codePoint)) {
       codePoint = input.codePointAt(i);
       if (codePoint < 0x20 || codePoint >= 0x7f
-          || PATH_SEGMENT_ENCODE_SET.indexOf(codePoint) != -1
-          || (codePoint == '%' && !alreadyEncoded)) {
+          || PATH_SEGMENT_ALWAYS_ENCODE_SET.indexOf(codePoint) != -1
+          || (!alreadyEncoded && (codePoint == '/' || codePoint == '%'))) {
         // Slow path: the character at i requires encoding!
         Buffer out = new Buffer();
         out.writeUtf8(input, 0, i);
-        canonicalize(out, input, i, limit, alreadyEncoded);
+        canonicalizeForPath(out, input, i, limit, alreadyEncoded);
         return out.readUtf8();
       }
     }
@@ -107,7 +107,7 @@ private static String canonicalize(String input, boolean alreadyEncoded) {
     return input;
   }
 
-  private static void canonicalize(Buffer out, String input, int pos, int limit,
+  private static void canonicalizeForPath(Buffer out, String input, int pos, int limit,
       boolean alreadyEncoded) {
     Buffer utf8Buffer = null; // Lazily allocated.
     int codePoint;
@@ -116,10 +116,9 @@ private static void canonicalize(Buffer out, String input, int pos, int limit,
       if (alreadyEncoded
           && (codePoint == '\t' || codePoint == '\n' || codePoint == '\f' || codePoint == '\r')) {
         // Skip this character.
-      } else if (codePoint < 0x20
-          || codePoint >= 0x7f
-          || PATH_SEGMENT_ENCODE_SET.indexOf(codePoint) != -1
-          || (codePoint == '%' && !alreadyEncoded)) {
+      } else if (codePoint < 0x20 || codePoint >= 0x7f
+          || PATH_SEGMENT_ALWAYS_ENCODE_SET.indexOf(codePoint) != -1
+          || (!alreadyEncoded && (codePoint == '/' || codePoint == '%'))) {
         // Percent encode this character.
         if (utf8Buffer == null) {
           utf8Buffer = new Buffer();

diff --git a/retrofit/src/test/java/retrofit2/RequestBuilderTest.java b/retrofit/src/test/java/retrofit2/RequestBuilderTest.java
@@ -670,6 +670,48 @@ Call<ResponseBody> method(@Path(value = "ping", encoded = true) String ping) {
     assertThat(request.body()).isNull();
   }
 
+  @Test public void getWithEncodedPathSegments() {
+    class Example {
+      @GET("/foo/bar/{ping}/") //
+      Call<ResponseBody> method(@Path(value = "ping", encoded = true) String ping) {
+        return null;
+      }
+    }
+    Request request = buildRequest(Example.class, "baz/pong/more");
+    assertThat(request.method()).isEqualTo("GET");
+    assertThat(request.headers().size()).isZero();
+    assertThat(request.url().toString()).isEqualTo("http://example.com/foo/bar/baz/pong/more/");
+    assertThat(request.body()).isNull();
+  }
+
+  @Test public void getWithUnencodedPathSegmentsPreventsRequestSplitting() {
+    class Example {
+      @GET("/foo/bar/{ping}/") //
+      Call<ResponseBody> method(@Path(value = "ping", encoded = false) String ping) {
+        return null;
+      }
+    }
+    Request request = buildRequest(Example.class, "baz/\r\nheader: blue");
+    assertThat(request.method()).isEqualTo("GET");
+    assertThat(request.headers().size()).isZero();
+    assertThat(request.url().toString()).isEqualTo("http://example.com/foo/bar/baz%2F%0D%0Aheader:%20blue/");
+    assertThat(request.body()).isNull();
+  }
+
+  @Test public void getWithEncodedPathStillPreventsRequestSplitting() {
+    class Example {
+      @GET("/foo/bar/{ping}/") //
+      Call<ResponseBody> method(@Path(value = "ping", encoded = true) String ping) {
+        return null;
+      }
+    }
+    Request request = buildRequest(Example.class, "baz/\r\npong");
+    assertThat(request.method()).isEqualTo("GET");
+    assertThat(request.headers().size()).isZero();
+    assertThat(request.url().toString()).isEqualTo("http://example.com/foo/bar/baz/pong/");
+    assertThat(request.body()).isNull();
+  }
+
   @Test public void pathParamRequired() {
     class Example {
       @GET("/foo/bar/{ping}/") //