Fix FormEncodingBuilder to be consistent with browsers.

Right now we're using the same encode set as the URL query; that's a mistake.
Rainmonth · Jun 21, 2015 · 33c5211 · 33c5211
1 parent cf4b019
commit 33c5211
Show file tree

Hide file tree

Showing 3 changed files with 96 additions and 6 deletions.
diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/FormEncodingBuilderTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/FormEncodingBuilderTest.java
@@ -15,6 +15,7 @@
  */
 package com.squareup.okhttp;
 
+import java.io.IOException;
 import okio.Buffer;
 import org.junit.Test;
 
@@ -30,7 +31,7 @@ public final class FormEncodingBuilderTest {
 
     assertEquals("application/x-www-form-urlencoded", formEncoding.contentType().toString());
 
-    String expected = "a%26b=c%3Dd&space,%20the=final%20frontier&%2525=%2525";
+    String expected = "a%26b=c%3Dd&space%2C%20the=final%20frontier&%2525=%2525";
     assertEquals(expected.length(), formEncoding.contentLength());
 
     Buffer out = new Buffer();
@@ -89,4 +90,92 @@ public final class FormEncodingBuilderTest {
     formEncoding.writeTo(buffer);
     assertEquals(expected, buffer.readUtf8());
   }
+
+  @Test public void characterEncoding() throws Exception {
+    assertEquals("%00", formEncode(0)); // Browsers convert '\u0000' to '%EF%BF%BD'.
+    assertEquals("%01", formEncode(1));
+    assertEquals("%02", formEncode(2));
+    assertEquals("%03", formEncode(3));
+    assertEquals("%04", formEncode(4));
+    assertEquals("%05", formEncode(5));
+    assertEquals("%06", formEncode(6));
+    assertEquals("%07", formEncode(7));
+    assertEquals("%08", formEncode(8));
+    assertEquals("%09", formEncode(9));
+    assertEquals("%0A", formEncode(10)); // Browsers convert '\n' to '\r\n'
+    assertEquals("%0B", formEncode(11));
+    assertEquals("%0C", formEncode(12));
+    assertEquals("%0D", formEncode(13)); // Browsers convert '\r' to '\r\n'
+    assertEquals("%0E", formEncode(14));
+    assertEquals("%0F", formEncode(15));
+    assertEquals("%10", formEncode(16));
+    assertEquals("%11", formEncode(17));
+    assertEquals("%12", formEncode(18));
+    assertEquals("%13", formEncode(19));
+    assertEquals("%14", formEncode(20));
+    assertEquals("%15", formEncode(21));
+    assertEquals("%16", formEncode(22));
+    assertEquals("%17", formEncode(23));
+    assertEquals("%18", formEncode(24));
+    assertEquals("%19", formEncode(25));
+    assertEquals("%1A", formEncode(26));
+    assertEquals("%1B", formEncode(27));
+    assertEquals("%1C", formEncode(28));
+    assertEquals("%1D", formEncode(29));
+    assertEquals("%1E", formEncode(30));
+    assertEquals("%1F", formEncode(31));
+    assertEquals("%20", formEncode(32)); // Browsers use '+' for space.
+    assertEquals("%21", formEncode(33));
+    assertEquals("%22", formEncode(34));
+    assertEquals("%23", formEncode(35));
+    assertEquals("%24", formEncode(36));
+    assertEquals("%25", formEncode(37));
+    assertEquals("%26", formEncode(38));
+    assertEquals("%27", formEncode(39));
+    assertEquals("%28", formEncode(40));
+    assertEquals("%29", formEncode(41));
+    assertEquals("*", formEncode(42));
+    assertEquals("%2B", formEncode(43));
+    assertEquals("%2C", formEncode(44));
+    assertEquals("-", formEncode(45));
+    assertEquals(".", formEncode(46));
+    assertEquals("%2F", formEncode(47));
+    assertEquals("0", formEncode(48));
+    assertEquals("9", formEncode(57));
+    assertEquals("%3A", formEncode(58));
+    assertEquals("%3B", formEncode(59));
+    assertEquals("%3C", formEncode(60));
+    assertEquals("%3D", formEncode(61));
+    assertEquals("%3E", formEncode(62));
+    assertEquals("%3F", formEncode(63));
+    assertEquals("%40", formEncode(64));
+    assertEquals("A", formEncode(65));
+    assertEquals("Z", formEncode(90));
+    assertEquals("%5B", formEncode(91));
+    assertEquals("%5C", formEncode(92));
+    assertEquals("%5D", formEncode(93));
+    assertEquals("%5E", formEncode(94));
+    assertEquals("_", formEncode(95));
+    assertEquals("%60", formEncode(96));
+    assertEquals("a", formEncode(97));
+    assertEquals("z", formEncode(122));
+    assertEquals("%7B", formEncode(123));
+    assertEquals("%7C", formEncode(124));
+    assertEquals("%7D", formEncode(125));
+    assertEquals("%7E", formEncode(126));
+    assertEquals("%7F", formEncode(127));
+    assertEquals("%C2%80", formEncode(128));
+    assertEquals("%C3%BF", formEncode(255));
+  }
+
+  private String formEncode(int codePoint) throws IOException {
+    // Wrap the codepoint with regular printable characters to prevent trimming.
+    RequestBody body = new FormEncodingBuilder()
+        .add("a", new String(new int[] { 'b', codePoint, 'c' }, 0, 3))
+        .build();
+    Buffer buffer = new Buffer();
+    body.writeTo(buffer);
+    buffer.skip(3); // Skip "a=b" prefix.
+    return buffer.readUtf8(buffer.size() - 1); // Skip the "c" suffix.
+  }
 }
diff --git a/okhttp/src/main/java/com/squareup/okhttp/FormEncodingBuilder.java b/okhttp/src/main/java/com/squareup/okhttp/FormEncodingBuilder.java
@@ -33,10 +33,10 @@ public FormEncodingBuilder add(String name, String value) {
       content.writeByte('&');
     }
     HttpUrl.canonicalize(content, name, 0, name.length(),
-        HttpUrl.QUERY_COMPONENT_ENCODE_SET, false, true);
+        HttpUrl.FORM_ENCODE_SET, false, true);
     content.writeByte('=');
     HttpUrl.canonicalize(content, value, 0, value.length(),
-        HttpUrl.QUERY_COMPONENT_ENCODE_SET, false, true);
+        HttpUrl.FORM_ENCODE_SET, false, true);
     return this;
   }
 
@@ -46,10 +46,10 @@ public FormEncodingBuilder addEncoded(String name, String value) {
       content.writeByte('&');
     }
     HttpUrl.canonicalize(content, name, 0, name.length(),
-        HttpUrl.QUERY_COMPONENT_ENCODE_SET, true, true);
+        HttpUrl.FORM_ENCODE_SET, true, true);
     content.writeByte('=');
     HttpUrl.canonicalize(content, value, 0, value.length(),
-        HttpUrl.QUERY_COMPONENT_ENCODE_SET, true, true);
+        HttpUrl.FORM_ENCODE_SET, true, true);
     return this;
   }
 

diff --git a/okhttp/src/main/java/com/squareup/okhttp/HttpUrl.java b/okhttp/src/main/java/com/squareup/okhttp/HttpUrl.java
@@ -256,10 +256,11 @@ public final class HttpUrl {
   private static final char[] HEX_DIGITS =
       { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' };
   static final String USERNAME_ENCODE_SET = " \"':;<=>@[]^`{}|/\\?#";
-  static final String PASSWORD_ENCODE_SET = " \"':;<=>@[]\\^`{}|/\\?#";
+  static final String PASSWORD_ENCODE_SET = " \"':;<=>@[]^`{}|/\\?#";
   static final String PATH_SEGMENT_ENCODE_SET = " \"<>^`{}|/\\?#";
   static final String QUERY_ENCODE_SET = " \"'<>#";
   static final String QUERY_COMPONENT_ENCODE_SET = " \"'<>#&=";
+  static final String FORM_ENCODE_SET = " \"':;<=>@[]^`{}|/\\?#&!$(),~";
   static final String FRAGMENT_ENCODE_SET = "";
 
   /** Either "http" or "https". */