Fix form url encoding when charset is not UTF-8, close #1444

Motivation:

form urlencoding doesn’t properly honor charset. It uses it for
converting the bytes while those are supposed to be already in the
US-ASCII range. It should be using it the first encode into bytes,
which should be then escaped.

Modifications:

Use current optimized code for UTF-8 and fall back to URLEncoder for
other charsets.

Results:

Proper encoding when charset is different from UTF-8, eg GBK

Author: slandelle

client/src/main/java/org/asynchttpclient/util/HttpUtils.java

@@ -12,9 +12,11 @@
  */
 package org.asynchttpclient.util;
 
-import static java.nio.charset.StandardCharsets.ISO_8859_1;
+import static java.nio.charset.StandardCharsets.*;
 import static org.asynchttpclient.util.MiscUtils.isNonEmpty;
 
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
 import java.nio.ByteBuffer;
 import java.nio.charset.Charset;
 import java.util.List;
@@ -82,28 +84,41 @@ public static boolean followRedirect(AsyncHttpClientConfig config, Request reque
         return request.getFollowRedirect() != null ? request.getFollowRedirect() : config.isFollowRedirect();
     }
 
-    private static StringBuilder urlEncodeFormParams0(List<Param> params) {
+    public static ByteBuffer urlEncodeFormParams(List<Param> params, Charset charset) {
+        return StringUtils.charSequence2ByteBuffer(urlEncodeFormParams0(params, charset), US_ASCII);
+    }
+
+    private static StringBuilder urlEncodeFormParams0(List<Param> params, Charset charset) {
         StringBuilder sb = StringBuilderPool.DEFAULT.stringBuilder();
         for (Param param : params) {
-            encodeAndAppendFormParam(sb, param.getName(), param.getValue());
+            encodeAndAppendFormParam(sb, param.getName(), param.getValue(), charset);
         }
         sb.setLength(sb.length() - 1);
         return sb;
     }
 
-    public static ByteBuffer urlEncodeFormParams(List<Param> params, Charset charset) {
-        return StringUtils.charSequence2ByteBuffer(urlEncodeFormParams0(params), charset);
-    }
-
-    private static void encodeAndAppendFormParam(final StringBuilder sb, final CharSequence name, final CharSequence value) {
-        Utf8UrlEncoder.encodeAndAppendFormElement(sb, name);
+    private static void encodeAndAppendFormParam(StringBuilder sb, String name, String value, Charset charset) {
+        encodeAndAppendFormField(sb, name, charset);
         if (value != null) {
             sb.append('=');
-            Utf8UrlEncoder.encodeAndAppendFormElement(sb, value);
+            encodeAndAppendFormField(sb, value, charset);
         }
         sb.append('&');
     }
 
+    private static void encodeAndAppendFormField(StringBuilder sb, String field, Charset charset) {
+        if (charset.equals(UTF_8)) {
+            Utf8UrlEncoder.encodeAndAppendFormElement(sb, field);
+        } else {
+            try {
+                // TODO there's probably room for perf improvements
+                sb.append(URLEncoder.encode(field, charset.name()));
+            } catch (UnsupportedEncodingException e) {
+                // can't happen, as Charset was already resolved
+            }
+        }
+    }
+
     public static String hostHeader(Request request, Uri uri) {
         String virtualHost = request.getVirtualHost();
         if (virtualHost != null)

client/src/test/java/org/asynchttpclient/util/HttpUtilsTest.java

@@ -13,14 +13,23 @@
  */
 package org.asynchttpclient.util;
 
+import static java.nio.charset.StandardCharsets.*;
 import static org.testng.Assert.*;
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
 
+import java.net.URLEncoder;
+import java.nio.ByteBuffer;
+import java.nio.charset.CharacterCodingException;
 import java.nio.charset.Charset;
-import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
 
 import org.asynchttpclient.DefaultAsyncHttpClientConfig;
 import org.asynchttpclient.Dsl;
+import org.asynchttpclient.Param;
 import org.asynchttpclient.Request;
+import org.asynchttpclient.netty.util.ByteBufUtils;
 import org.asynchttpclient.uri.Uri;
 import org.testng.annotations.Test;
 
@@ -92,19 +101,19 @@ public void testIsSameBaseUrlReturnsTrueWhenOneUriHasDefaultPort() {
     @Test
     public void testParseCharsetWithoutQuotes() {
         Charset charset = HttpUtils.parseCharset("Content-type: application/json; charset=utf-8");
-        assertEquals(charset, StandardCharsets.UTF_8, "parseCharset returned wrong Charset");
+        assertEquals(charset, UTF_8, "parseCharset returned wrong Charset");
     }
 
     @Test
     public void testParseCharsetWithSingleQuotes() {
         Charset charset = HttpUtils.parseCharset("Content-type: application/json; charset='utf-8'");
-        assertEquals(charset, StandardCharsets.UTF_8, "parseCharset returned wrong Charset");
+        assertEquals(charset, UTF_8, "parseCharset returned wrong Charset");
     }
 
     @Test
     public void testParseCharsetWithDoubleQuotes() {
         Charset charset = HttpUtils.parseCharset("Content-type: application/json; charset=\"utf-8\"");
-        assertEquals(charset, StandardCharsets.UTF_8, "parseCharset returned wrong Charset");
+        assertEquals(charset, UTF_8, "parseCharset returned wrong Charset");
     }
 
     @Test
@@ -160,4 +169,34 @@ public void testGetFollowRedirectPriorityGivenToRequest() {
         boolean followRedirect = HttpUtils.followRedirect(config, request);
         assertFalse(followRedirect, "Follow redirect value set in request should be given priority");
     }
+
+    private void formUrlEncoding(Charset charset) throws Exception {
+        String key = "key";
+        String value = "中文";
+        List<Param> params = new ArrayList<>();
+        params.add(new Param(key, value));
+        ByteBuffer ahcBytes = HttpUtils.urlEncodeFormParams(params, charset);
+        String ahcString = toUsAsciiString(ahcBytes);
+        String jdkString = key + "=" + URLEncoder.encode(value, charset.name());
+        assertEquals(ahcString, jdkString);
+    }
+
+    @Test
+    public void formUrlEncodingShouldSupportUtf8Charset() throws Exception {
+        formUrlEncoding(UTF_8);
+    }
+
+    @Test
+    public void formUrlEncodingShouldSupportNonUtf8Charset() throws Exception {
+        formUrlEncoding(Charset.forName("GBK"));
+    }
+
+    private static String toUsAsciiString(ByteBuffer buf) throws CharacterCodingException {
+        ByteBuf bb = Unpooled.wrappedBuffer(buf);
+        try {
+            return ByteBufUtils.byteBuf2String(US_ASCII, bb);
+        } finally {
+            bb.release();
+        }
+    }
 }