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/

2017-03-31 Thread Mark Thomas 
On 31/03/17 14:45, Violeta Georgieva wrote:
> Hi Mark,
> 
> 2017-03-31 16:37 GMT+03:00 :
>>
>> Author: markt
>> Date: Fri Mar 31 13:37:32 2017
>> New Revision: 1789685
>>
>> URL: http://svn.apache.org/viewvc?rev=1789685=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=1789684=1789685=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

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/

2017-03-31 Thread Violeta Georgieva 
Hi Mark,

2017-03-31 16:37 GMT+03:00 :
>
> Author: markt
> Date: Fri Mar 31 13:37:32 2017
> New Revision: 1789685
>
> URL: http://svn.apache.org/viewvc?rev=1789685=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=1789684=1789685=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=1789684=1789685=diff
>

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/chan

2017-03-31 Thread markt 
Author: markt
Date: Fri Mar 31 13:37:32 2017
New Revision: 1789685

URL: http://svn.apache.org/viewvc?rev=1789685=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=1789684=1789685=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=1789684=1789685=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 @@
 
 
 
-
-  
-  
-  
-  
-  
-  
-
-
-  
-  
-  
-  
-  
-  
-
-
-