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? 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 >
