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: [email protected]
For additional commands, e-mail: [email protected]