On 31/03/17 14:45, Violeta Georgieva wrote: > Hi Mark, > > 2017-03-31 16:37 GMT+03:00 <ma...@apache.org>: >> >> 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 > > Isn't the change in mvn-pub.xml for some other issue?
<expletive /> Sorry. I'll revert that (I'm working on being able to publish snapshots without GPG signatures so we can publish snapshots from buildbot). Mark > > Regards, > Violeta > >> 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 >> > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
