You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2017/03/31 13:37:32 UTC
svn commit: r1789685 - in /tomcat/trunk:
java/org/apache/tomcat/util/http/parser/HttpParser.java res/maven/mvn-pub.xml
test/org/apache/tomcat/util/http/parser/TestHttpParserHost.java
webapps/docs/changelog.xml
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1789685 - in /tomcat/trunk:
java/org/apache/tomcat/util/http/parser/HttpParser.java res/maven/mvn-pub.xml
test/org/apache/tomcat/util/http/parser/TestHttpParserHost.java
webapps/docs/changelog.xml
Posted by Mark Thomas <ma...@apache.org>.
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-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1789685 - in /tomcat/trunk: java/org/apache/tomcat/util/http/parser/HttpParser.java
res/maven/mvn-pub.xml test/org/apache/tomcat/util/http/parser/TestHttpParserHost.java
webapps/docs/changelog.xml
Posted by Violeta Georgieva <mi...@gmail.com>.
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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>