Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=50721

Correctly handle URL decoding where the URL ends in %nn.
Patch (for fix) provided by Christof Marti.
Additional test cases added.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1069170 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/java/org/apache/catalina/util/RequestUtil.java b/java/org/apache/catalina/util/RequestUtil.java
index 1aab539..9333d95 100644 (file)
--- a/java/org/apache/catalina/util/RequestUtil.java
+++ b/java/org/apache/catalina/util/RequestUtil.java
@@ -326,7 +326,7 @@ public final class RequestUtil {
             if (b == '+' && isQuery) {
                 b = (byte)' ';
             } else if (b == '%') {
-                if (ix + 2 >= len) {
+                if (ix + 2 > len) {
                     throw new IllegalArgumentException(
                             sm.getString("requestUtil.urlDecode.missingDigit"));
                 }

diff --git a/test/org/apache/catalina/util/TestRequestUtil.java b/test/org/apache/catalina/util/TestRequestUtil.java
index 8633d93..5f48e9d 100644 (file)
--- a/test/org/apache/catalina/util/TestRequestUtil.java
+++ b/test/org/apache/catalina/util/TestRequestUtil.java
@@ -28,7 +28,7 @@ public class TestRequestUtil extends TestCase {
         assertEquals("/",RequestUtil.normalize("//"));
     }
 
-    public void testURLDecodeString() {
+    public void testURLDecodeStringInvalid() {
         // %n rather than %nn should throw an IAE according to the Javadoc
         Exception exception = null;
         try {
@@ -47,4 +47,40 @@ public class TestRequestUtil extends TestCase {
         }
         assertTrue(exception instanceof IllegalArgumentException);
     }
+    
+    public void testURLDecodeStringValidIso88591Start() {
+
+        String result = RequestUtil.URLDecode("%41xxxx", "ISO-8859-1");
+        assertEquals("Axxxx", result);
+    }
+
+    public void testURLDecodeStringValidIso88591Middle() {
+
+        String result = RequestUtil.URLDecode("xx%41xx", "ISO-8859-1");
+        assertEquals("xxAxx", result);
+    }
+
+    public void testURLDecodeStringValidIso88591End() {
+
+        String result = RequestUtil.URLDecode("xxxx%41", "ISO-8859-1");
+        assertEquals("xxxxA", result);
+    }
+
+    public void testURLDecodeStringValidUtf8Start() {
+        String result = RequestUtil.URLDecode("%c3%aaxxxx", "UTF-8");
+        assertEquals("\u00eaxxxx", result);
+    }
+
+    public void testURLDecodeStringValidUtf8Middle() {
+
+        String result = RequestUtil.URLDecode("xx%c3%aaxx", "UTF-8");
+        assertEquals("xx\u00eaxx", result);
+    }
+
+    public void testURLDecodeStringValidUtf8End() {
+
+        String result = RequestUtil.URLDecode("xxxx%c3%aa", "UTF-8");
+        assertEquals("xxxx\u00ea", result);
+    }
+
 }

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 01740cb..d6a8dd2 100644 (file)
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -64,8 +64,12 @@
         the expected state transitions. (markt)
       </add>
       <fix>
+        <bug>50721</bug>: Correctly handle URL decoding where the URL ends in
+        %nn. Patch provided by Christof Marti. (markt)
+      </fix>
+      <fix>
         <bug>50748</bug>: Allow the content length header to be set up to the
-        point the response is committed when a writer is beng used. (markt)
+        point the response is committed when a writer is being used. (markt)
       </fix>
     </changelog>
   </subsection>