UNDERTOW-818 Fix issue with URL encoding not being handled correctly …

…when subsequent characters in a multi byte character are not percent encoded
undertow-io · Sep 2, 2016 · d53c659 · d53c659
1 parent c1c810f
commit d53c659
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 38 deletions.
diff --git a/core/src/main/java/io/undertow/util/URLUtils.java b/core/src/main/java/io/undertow/util/URLUtils.java
@@ -92,54 +92,65 @@ public static String decode(String s, String enc, boolean decodeSlash, StringBui
                  * consecutive  bytes obtained this way to whatever
                  * character(s) they represent in the provided
                  * encoding.
+                 *
+                 * Note that we need to decode the whole rest of the value, we can't just decode
+                 * three characters. For multi code point characters there if the code point can be
+                 * represented as an alphanumeric
                  */
                     try {
-                        // (numChars-i)/3 is an upper bound for the number
+                        // (numChars-i) is an upper bound for the number
                         // of remaining bytes
                         if (bytes == null) {
-                            bytes = new byte[(numChars - i) / 3];
+                            bytes = new byte[numChars - i + 1];
                         }
                         int pos = 0;
 
-                        while (((i + 2) < numChars) && (c == '%')) {
-                            char p1 = Character.toLowerCase(s.charAt(i + 1));
-                            char p2 = Character.toLowerCase(s.charAt(i + 2));
-                            int v = 0;
-                            if (p1 >= '0' && p1 <= '9') {
-                                v = (p1 - '0') << 4;
-                            } else if (p1 >= 'a' && p1 <= 'f') {
-                                v = (p1 - 'a' + 10) << 4;
-                            } else {
-                                throw UndertowMessages.MESSAGES.failedToDecodeURL(s, enc, null);
-                            }
-                            if (p2 >= '0' && p2 <= '9') {
-                                v += (p2 - '0');
-                            } else if (p2 >= 'a' && p2 <= 'f') {
-                                v += (p2 - 'a' + 10);
-                            } else {
-                                throw UndertowMessages.MESSAGES.failedToDecodeURL(s, enc, null);
-                            }
-                            if (v < 0) {
-                                throw UndertowMessages.MESSAGES.failedToDecodeURL(s, enc, null);
-                            }
-                            if(v == '/' || v== '\\') {
-                                mightRequireSlashEscape = true;
-                            }
+                        while ((i< numChars)) {
+                            if (c == '%') {
+                                char p1 = Character.toLowerCase(s.charAt(i + 1));
+                                char p2 = Character.toLowerCase(s.charAt(i + 2));
+                                int v = 0;
+                                if (p1 >= '0' && p1 <= '9') {
+                                    v = (p1 - '0') << 4;
+                                } else if (p1 >= 'a' && p1 <= 'f') {
+                                    v = (p1 - 'a' + 10) << 4;
+                                } else {
+                                    throw UndertowMessages.MESSAGES.failedToDecodeURL(s, enc, null);
+                                }
+                                if (p2 >= '0' && p2 <= '9') {
+                                    v += (p2 - '0');
+                                } else if (p2 >= 'a' && p2 <= 'f') {
+                                    v += (p2 - 'a' + 10);
+                                } else {
+                                    throw UndertowMessages.MESSAGES.failedToDecodeURL(s, enc, null);
+                                }
+                                if (v < 0) {
+                                    throw UndertowMessages.MESSAGES.failedToDecodeURL(s, enc, null);
+                                }
+                                if (v == '/' || v == '\\') {
+                                    mightRequireSlashEscape = true;
+                                }
 
-                            bytes[pos++] = (byte) v;
-                            i += 3;
-                            if (i < numChars) {
-                                c = s.charAt(i);
+                                bytes[pos++] = (byte) v;
+                                i += 3;
+                                if (i < numChars) {
+                                    c = s.charAt(i);
+                                }
+                            }else if(c == '+') {
+                                bytes[pos++] = (byte) ' ';
+                                ++i;
+                                if (i < numChars) {
+                                    c = s.charAt(i);
+                                }
+                            } else {
+                                bytes[pos++] = (byte) c;
+                                ++i;
+                                if (i < numChars) {
+                                    c = s.charAt(i);
+                                }
                             }
                         }
 
-                        // A trailing, incomplete byte encoding such as
-                        // "%x" will cause an exception to be thrown
-
-                        if ((i < numChars) && (c == '%')) {
-                            throw UndertowMessages.MESSAGES.failedToDecodeURL(s, enc, null);
-                        }
-
                         String decoded = new String(bytes, 0, pos, enc);
                         if (!decodeSlash && mightRequireSlashEscape) {
                             //we need to re-encode slash characters

diff --git a/core/src/test/java/io/undertow/server/handlers/QueryParametersTestCase.java b/core/src/test/java/io/undertow/server/handlers/QueryParametersTestCase.java
@@ -23,6 +23,7 @@
 import java.util.Iterator;
 import java.util.Map;
 
+import io.undertow.UndertowOptions;
 import io.undertow.server.HttpHandler;
 import io.undertow.server.HttpServerExchange;
 import io.undertow.testutils.DefaultServer;
@@ -33,6 +34,7 @@
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.xnio.OptionMap;
 
 /**
  * Tests that query parameters are handled correctly.
@@ -91,12 +93,30 @@ public void testQueryParameters() throws IOException {
             runTest(client, "{a=>b,s =>,t =>,value=>[bb,cc]}", "/path?a=b&value=bb&value=cc&s%20&t%20&");
             runTest(client, "{a=>b,s =>,t =>,u=>,value=>[bb,cc]}", "/path?a=b&value=bb&value=cc&s%20&t%20&u");
 
-
         } finally {
             client.getConnectionManager().shutdown();
         }
     }
 
+
+
+    @Test
+    public void testQueryParametersShiftJIS() throws IOException {
+        OptionMap old = DefaultServer.getUndertowOptions();
+        try {
+            DefaultServer.setUndertowOptions(OptionMap.create(UndertowOptions.URL_CHARSET, "Shift_JIS"));
+            TestHttpClient client = new TestHttpClient();
+            try {
+                runTest(client, "{unicode=>テスト}", "/path?unicode=%83e%83X%83g");
+
+            } finally {
+                client.getConnectionManager().shutdown();
+            }
+        } finally {
+            DefaultServer.setUndertowOptions(old);
+        }
+    }
+
     private void runTest(final TestHttpClient client, final String expected, final String queryString) throws IOException {
         Assert.assertEquals(expected, HttpClientUtils.readResponse(client.execute(new HttpGet(DefaultServer.getDefaultServerURL() + queryString))));
     }