Author: markt
Date: Fri Mar 31 13:37:32 2017
New Revision: 1789685

URL: http://svn.apache.org/viewvc?rev=1789685&view=rev
Log:
Correct various edge cases in the new HTTP Host header validation parser.
Patch provided by Katya Todorova.
This closes #48

Fix IPv6/IPv4 parsing for host header:
    - chars other than : should not be allowed in IPv6 address after ]
    - ::: should not present in IPv6 address
    - IPv4 part of IPv6 address was not correctly parsed (1 symbol of IPv4 part 
was ignored)
    - tests added to cover IPv4/6 parsing 
    - parsed test class fixed not to throw NPE when an exception is expected 
but not thrown 

Modified:
    tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
    tomcat/trunk/res/maven/mvn-pub.xml
    tomcat/trunk/test/org/apache/tomcat/util/http/parser/TestHttpParserHost.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java?rev=1789685&r1=1789684&r2=1789685&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java 
(original)
+++ tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java Fri 
Mar 31 13:37:32 2017
@@ -553,45 +553,50 @@ public class HttpParser {
         int h16Size = 0;
         int pos = 1;
         boolean parsedDoubleColon = false;
-        boolean previousWasColon = false;
+        int precedingColonsCount = 0;
 
         do {
             c = reader.read();
-            if (h16Count == 0 && previousWasColon && c != ':') {
+            if (h16Count == 0 && precedingColonsCount == 1 && c != ':') {
                 // Can't start with a single :
                 throw new IllegalArgumentException();
             }
             if (HttpParser.isHex(c)) {
                 if (h16Size == 0) {
                     // Start of a new h16 block
-                    previousWasColon = false;
+                    precedingColonsCount = 0;
                     h16Count++;
-                    reader.mark(4);
                 }
                 h16Size++;
                 if (h16Size > 4) {
                     throw new IllegalArgumentException();
                 }
             } else if (c == ':') {
-                if (previousWasColon) {
-                    // End of ::
-                    if (parsedDoubleColon) {
-                        // Only allowed one :: sequence
-                        throw new IllegalArgumentException();
-                    }
-                    parsedDoubleColon = true;
-                    previousWasColon = false;
-                    // :: represents at least one h16 block
-                    h16Count++;
+                if (precedingColonsCount >=2 ) {
+                    // ::: is not allowed
+                    throw new IllegalArgumentException();
                 } else {
-                    previousWasColon = true;
+                    if(precedingColonsCount == 1) {
+                        // End of ::
+                        if (parsedDoubleColon ) {
+                            // Only allowed one :: sequence
+                            throw new IllegalArgumentException();
+                        }
+                        parsedDoubleColon = true;
+                        // :: represents at least one h16 block
+                        h16Count++;
+                    }
+                    precedingColonsCount++;
+                    // mark if the next symbol is hex before the actual read
+                    reader.mark(4);
                 }
                 h16Size = 0;
             } else if (c == ']') {
-                if (previousWasColon) {
+                if (precedingColonsCount == 1) {
                     // Can't end on a single ':'
                     throw new IllegalArgumentException();
                 }
+                pos++;
                 break;
             } else if (c == '.') {
                 if (h16Count == 7 || h16Count < 7 && parsedDoubleColon) {
@@ -617,9 +622,12 @@ public class HttpParser {
 
         c = reader.read();
         if (c == ':') {
-            return pos + 1;
+            return pos;
         } else {
-            return -1;
+            if(c == -1) {
+                return -1;
+            }
+            throw new IllegalArgumentException();
         }
     }
 

Modified: tomcat/trunk/res/maven/mvn-pub.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/res/maven/mvn-pub.xml?rev=1789685&r1=1789684&r2=1789685&view=diff
==============================================================================
--- tomcat/trunk/res/maven/mvn-pub.xml (original)
+++ tomcat/trunk/res/maven/mvn-pub.xml Fri Mar 31 13:37:32 2017
@@ -49,35 +49,18 @@
     </copy>
 
     <!--sign the jar, the source and the pom -->
-    <exec executable="${gpg.exec}" failonerror="true"
-          inputstring="${gpg.passphrase}">
-      <arg value="--batch"/>
-      <arg value="--passphrase-fd"/>
-      <arg value="0"/>
-      <arg value="-a"/>
-      <arg value="-b"/>
-      <arg value="${file}"/>
-    </exec>
-    <exec executable="${gpg.exec}" failonerror="true"
-          inputstring="${gpg.passphrase}">
-      <arg value="--batch"/>
-      <arg value="--passphrase-fd"/>
-      <arg value="0"/>
-      <arg value="-a"/>
-      <arg value="-b"/>
-      <arg value="${src}"/>
-    </exec>
-    <exec executable="${gpg.exec}" failonerror="true"
-          inputstring="${gpg.passphrase}">
-      <arg value="--batch"/>
-      <arg value="--passphrase-fd"/>
-      <arg value="0"/>
-      <arg value="-a"/>
-      <arg value="-b"/>
-      <arg value="-o"/>
-      <arg value="${pom}.asc"/>
-      <arg value="${pom}.tmp"/>
-    </exec>
+    <antcall target="-sign" >
+      <param name="file.in" value="@{file}" />
+      <param name="file.out" value="@{file}.asc" />
+    </antcall>
+    <antcall target="-sign" >
+      <param name="file.in" value="@{src}" />
+      <param name="file.out" value="@{src}.asc" />
+    </antcall>
+    <antcall target="-sign" >
+      <param name="file.in" value="@{pom}.tmp" />
+      <param name="file.out" value="@{pom}.asc" />
+    </antcall>
 
     <artifact:deploy file="${file}">
         <pom file="${pom}.tmp"/>
@@ -131,26 +114,14 @@
     </copy>
 
     <!--sign the file and pom -->
-    <exec executable="${gpg.exec}" failonerror="true"
-          inputstring="${gpg.passphrase}">
-      <arg value="--batch"/>
-      <arg value="--passphrase-fd"/>
-      <arg value="0"/>
-      <arg value="-a"/>
-      <arg value="-b"/>
-      <arg value="${file}"/>
-    </exec>
-    <exec executable="${gpg.exec}" failonerror="true"
-          inputstring="${gpg.passphrase}">
-      <arg value="--batch"/>
-      <arg value="--passphrase-fd"/>
-      <arg value="0"/>
-      <arg value="-a"/>
-      <arg value="-b"/>
-      <arg value="-o"/>
-      <arg value="${pom}.asc"/>
-      <arg value="${pom}.tmp"/>
-    </exec>
+    <antcall target="-sign" >
+      <param name="file.in" value="@{file}" />
+      <param name="file.out" value="@{file}.asc" />
+    </antcall>
+    <antcall target="-sign" >
+      <param name="file.in" value="@{pom}.tmp" />
+      <param name="file.out" value="@{pom}.asc" />
+    </antcall>
 
     <artifact:deploy file="${file}">
       <pom file="${pom}.tmp"/>
@@ -198,35 +169,18 @@
     </copy>
 
     <!--sign the zip, the tar.gz and the pom -->
-    <exec executable="${gpg.exec}" failonerror="true"
-          inputstring="${gpg.passphrase}">
-      <arg value="--batch"/>
-      <arg value="--passphrase-fd"/>
-      <arg value="0"/>
-      <arg value="-a"/>
-      <arg value="-b"/>
-      <arg value="${file}.zip"/>
-    </exec>
-    <exec executable="${gpg.exec}" failonerror="true"
-          inputstring="${gpg.passphrase}">
-      <arg value="--batch"/>
-      <arg value="--passphrase-fd"/>
-      <arg value="0"/>
-      <arg value="-a"/>
-      <arg value="-b"/>
-      <arg value="${file}.tar.gz"/>
-    </exec>
-    <exec executable="${gpg.exec}" failonerror="true"
-          inputstring="${gpg.passphrase}">
-      <arg value="--batch"/>
-      <arg value="--passphrase-fd"/>
-      <arg value="0"/>
-      <arg value="-a"/>
-      <arg value="-b"/>
-      <arg value="-o"/>
-      <arg value="${pom}.asc"/>
-      <arg value="${pom}.tmp"/>
-    </exec>
+    <antcall target="-sign" >
+      <param name="file.in" value="@{file}" />
+      <param name="file.out" value="@{file}.asc" />
+    </antcall>
+    <antcall target="-sign" >
+      <param name="file.in" value="@{file}.tar.gz" />
+      <param name="file.out" value="@{file}.tar.gz.asc" />
+    </antcall>
+    <antcall target="-sign" >
+      <param name="file.in" value="@{pom}.tmp" />
+      <param name="file.out" value="@{pom}.asc" />
+    </antcall>
 
     <artifact:deploy file="${pom}">
         <pom file="${pom}.tmp"/>
@@ -262,7 +216,7 @@
     </sequential>
   </macrodef>
 
-  <target name="generic-deploy" depends="init-maven,init-gpg,init-ldap">
+  <target name="generic-deploy" 
depends="init-maven,init-gpg-1,init-gpg-2,init-ldap">
     <!-- Standard jars in bin directory -->
     <!-- Skip bootstrap.jar - it is just a subset of catalina.jar -->
     <doMavenDeploy artifactId="tomcat-juli"
@@ -399,7 +353,11 @@
     </antcall>
   </target>
 
-  <target name="init-gpg">
+  <target name="init-gpg-1">
+    <available file="${gpg.exec}" property="gpg.exec.available"/>
+  </target>
+
+  <target name="init-gpg-2" if="${gpg.exec.available}">
     <input message="Enter GPG pass-phrase" addproperty="gpg.passphrase" >
       <handler type="secure"/>
     </input>
@@ -412,4 +370,19 @@
     </input>
   </target>
 
+  <target name="-sign" if="gpg.passphrase">
+    <fail unless="file" />
+    <exec executable="${gpg.exec}" failonerror="true"
+          inputstring="${gpg.passphrase}">
+      <arg value="--batch"/>
+      <arg value="--passphrase-fd"/>
+      <arg value="0"/>
+      <arg value="-a"/>
+      <arg value="-b"/>
+      <arg value="-o"/>
+      <arg value="${file.out}"/>
+      <arg value="${file.in}"/>
+    </exec>
+  </target>
+
 </project>

Modified: 
tomcat/trunk/test/org/apache/tomcat/util/http/parser/TestHttpParserHost.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/http/parser/TestHttpParserHost.java?rev=1789685&r1=1789684&r2=1789685&view=diff
==============================================================================
--- 
tomcat/trunk/test/org/apache/tomcat/util/http/parser/TestHttpParserHost.java 
(original)
+++ 
tomcat/trunk/test/org/apache/tomcat/util/http/parser/TestHttpParserHost.java 
Fri Mar 31 13:37:32 2017
@@ -66,6 +66,7 @@ public class TestHttpParserHost {
         result.add(new Object[] { TestType.IPv4, "0.0.0.256", 
Integer.valueOf(-1), IAE} );
         result.add(new Object[] { TestType.IPv4, "0.a.0.0", 
Integer.valueOf(-1), IAE} );
         result.add(new Object[] { TestType.IPv4, "0..0.0", 
Integer.valueOf(-1), IAE} );
+        result.add(new Object[] { TestType.IPv4, "0]", Integer.valueOf(-1), 
IAE} );
         // Domain Name - valid
         result.add(new Object[] { TestType.DOMAIN_NAME, "localhost", 
Integer.valueOf(-1), null} );
         result.add(new Object[] { TestType.DOMAIN_NAME, "localhost:8080", 
Integer.valueOf(9), null} );
@@ -121,6 +122,7 @@ public class TestHttpParserHost {
         result.add(new Object[] { TestType.IPv6, "[0:0:0:0:0:0:127.0.0.1]", 
Integer.valueOf(-1), null} );
         result.add(new Object[] { TestType.IPv6, 
"[0:0:0:0:0:0:127.0.0.1]:8080",
                 Integer.valueOf(23), null} );
+        result.add(new Object[] { TestType.IPv6, "[::1.2.3.4]", 
Integer.valueOf(-1), null} );
         // IPv6 - invalid
         result.add(new Object[] { TestType.IPv6, 
"[1234:5678:90AB:CDEF:1234:127.0.0.1]",
                 Integer.valueOf(-1), IAE} );
@@ -136,6 +138,18 @@ public class TestHttpParserHost {
         result.add(new Object[] { TestType.IPv6, "[0::0::127.0.0.1]", 
Integer.valueOf(-1), IAE} );
         result.add(new Object[] { TestType.IPv6, "[0:0:G:0:0:0:127.0.0.1]", 
Integer.valueOf(-1), IAE} );
         result.add(new Object[] { TestType.IPv6, 
"[00000:0:0:0:0:0:127.0.0.1]", Integer.valueOf(-1), IAE} );
+        result.add(new Object[] { TestType.IPv6, "[::1]'", 
Integer.valueOf(-1), IAE} );
+        result.add(new Object[] { TestType.IPv6, 
"[:2222:3333:4444:5555:6666:7777:8888]",
+                Integer.valueOf(-1), IAE} );
+        result.add(new Object[] { TestType.IPv6, 
"[1111:::3333:4444:5555:6666:7777:8888]",
+                Integer.valueOf(-1), IAE} );
+        result.add(new Object[] { TestType.IPv6, "::1]", Integer.valueOf(-1), 
IAE} );
+        result.add(new Object[] { TestType.IPv6, 
"[1111:2222:3333:4444:5555:6666:7777:8888:9999]",
+                Integer.valueOf(-1), IAE} );
+        result.add(new Object[] { TestType.IPv6, 
"[1111:2222:3333:4444:5555:6666:7777:1.2.3.4]",
+            Integer.valueOf(-1), IAE} );
+        result.add(new Object[] { TestType.IPv6, "[1111:2222:3333]",
+            Integer.valueOf(-1), IAE} );
         return result;
     }
 
@@ -165,6 +179,7 @@ public class TestHttpParserHost {
         if (expectedException == null) {
             Assert.assertNull(input, exceptionClass);
         } else {
+            Assert.assertNotNull(exceptionClass);
             Assert.assertTrue(input, 
expectedException.isAssignableFrom(exceptionClass));
         }
     }

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1789685&r1=1789684&r2=1789685&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Fri Mar 31 13:37:32 2017
@@ -52,6 +52,10 @@
         method name from <code>getPushBuilder()</code> to
         <code>newPushBuilder()</code>. (markt)
       </update>
+      <fix>
+        Correct various edge cases in the new HTTP Host header validation
+        parser. Patch provided by Katya Todorova. (martk)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to