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