You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Rainer Jung <ra...@kippdata.de> on 2011/06/05 15:09:20 UTC
Re: svn commit: r1132362 - in /tomcat/trunk/java/org/apache/coyote/http11:
AbstractHttp11Processor.java Http11AprProcessor.java Http11NioProcessor.java
Http11Processor.java
Hi Mark,
On 05.06.2011 12:06, markt@apache.org wrote:
> Modified: tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1132362&r1=1132361&r2=1132362&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original)
> +++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Sun Jun 5 10:06:49 2011
> @@ -41,6 +41,7 @@ import org.apache.juli.logging.Log;
> import org.apache.tomcat.util.ExceptionUtils;
> import org.apache.tomcat.util.buf.Ascii;
> import org.apache.tomcat.util.buf.ByteChunk;
> +import org.apache.tomcat.util.buf.HexUtils;
> import org.apache.tomcat.util.buf.MessageBytes;
> import org.apache.tomcat.util.http.FastHttpDateFormat;
> import org.apache.tomcat.util.http.MimeHeaders;
> @@ -967,6 +968,78 @@ public abstract class AbstractHttp11Proc
>
> abstract boolean prepareSendfile(OutputFilter[] outputFilters);
>
> + /**
> + * Parse host.
> + */
> + protected void parseHost(MessageBytes valueMB) {
> +
> + if (valueMB == null || valueMB.isNull()) {
> + // HTTP/1.0
> + // If no host header, use the port info from the endpoint
> + // The host will be obtained lazily from the socket if required
> + // using ActionCode#REQ_LOCAL_NAME_ATTRIBUTE
> + request.setServerPort(endpoint.getPort());
> + return;
> + }
> +
> + ByteChunk valueBC = valueMB.getByteChunk();
> + byte[] valueB = valueBC.getBytes();
> + int valueL = valueBC.getLength();
> + int valueS = valueBC.getStart();
> + int colonPos = -1;
> + if (hostNameC.length < valueL) {
> + hostNameC = new char[valueL];
> + }
> +
> + boolean ipv6 = (valueB[valueS] == '[');
> + boolean bracketClosed = false;
> + for (int i = 0; i < valueL; i++) {
> + char b = (char) valueB[i + valueS];
> + hostNameC[i] = b;
> + if (b == ']') {
> + bracketClosed = true;
> + } else if (b == ':') {
> + if (!ipv6 || bracketClosed) {
> + colonPos = i;
> + break;
> + }
> + }
> + }
> +
> + if (colonPos < 0) {
> + if (!endpoint.isSSLEnabled()) {
> + // 80 - Default HTTP port
> + request.setServerPort(80);
> + } else {
> + // 443 - Default HTTPS port
> + request.setServerPort(443);
> + }
> + request.serverName().setChars(hostNameC, 0, valueL);
> + } else {
> +
> + request.serverName().setChars(hostNameC, 0, colonPos);
> +
> + int port = 0;
> + int mult = 1;
> + for (int i = valueL - 1; i > colonPos; i--) {
> + int charValue = HexUtils.getDec(valueB[i + valueS]);
Any idea, why hex digits (including a-f, A-F) are allowed in port numbers?
I know you only moved that code, but it reminded me of an observation I
made long ago and forgot.
> + if (charValue == -1) {
> + // Invalid character
> + error = true;
> + // 400 - Bad request
> + response.setStatus(400);
> + adapter.log(request, response, 0);
> + break;
> + }
> + port = port + (charValue * mult);
> + mult = 10 * mult;
> + }
> + request.setServerPort(port);
> +
> + }
> +
> + }
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1132362 - in /tomcat/trunk/java/org/apache/coyote/http11:
AbstractHttp11Processor.java Http11AprProcessor.java Http11NioProcessor.java
Http11Processor.java
Posted by Mark Thomas <ma...@apache.org>.
On 06/06/2011 06:54, Konstantin Kolinko wrote:
> 2011/6/6 Mark Thomas <ma...@apache.org>:
>> On 05/06/2011 17:47, Rainer Jung wrote:
>>>>>> + for (int i = valueL - 1; i > colonPos; i--) {
>>>>>> + int charValue = HexUtils.getDec(valueB[i + valueS]);
>>>>>
>>>>> Any idea, why hex digits (including a-f, A-F) are allowed in port numbers?
>
> Probably that was just to do not invent a separate method.
> Another way to fix this would be to replace the next line
> if (charValue == -1) {
> with
> if (charValue == -1 || charValue > 9) {
<snip/>
> Regarding the patch,
> http://svn.apache.org/viewvc?rev=1132487&view=rev
>
> Code looks OK but my small concern is that the invalid case is handled
> with a NumberFormatException here and it can be triggered by an
> external request. I have not tested it here, but general notion is
> that exception creation is expensive.
>
> Maybe we can use a cached copy of NumberFormatException, like
> o.a.naming.resources.ImmutableNameNotFoundException
I think reverting to the original code and using the change you
suggested above is a better solution. I'll do that shortly.
> By the way, there are two HexUtils classes in trunk. Do we need both?
I'll take a look and remove one if I can. Thanks for spotting it.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1132362 - in /tomcat/trunk/java/org/apache/coyote/http11:
AbstractHttp11Processor.java Http11AprProcessor.java Http11NioProcessor.java Http11Processor.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/6/6 Mark Thomas <ma...@apache.org>:
> On 05/06/2011 17:47, Rainer Jung wrote:
>>>>> + for (int i = valueL - 1; i > colonPos; i--) {
>>>>> + int charValue = HexUtils.getDec(valueB[i + valueS]);
>>>>
>>>> Any idea, why hex digits (including a-f, A-F) are allowed in port numbers?
Probably that was just to do not invent a separate method.
Another way to fix this would be to replace the next line
if (charValue == -1) {
with
if (charValue == -1 || charValue > 9) {
>>>> I know you only moved that code, but it reminded me of an observation I
>>>> made long ago and forgot.
>>>
>>> I can't think of any good reason. Happy to limit that to the digits 0-9.
>>
>> Would you prefer renaming HexUtils to NumberUtils (or similar) and havin
>> getDecFromHex() and getDecFromDec() there, or a new class similar to
>> HexUtils called DecUtils?
>
> Or we could just use Ascii.parseInt(char[], int, int)
>
> Just testing a patch now...
Regarding the patch,
http://svn.apache.org/viewvc?rev=1132487&view=rev
Code looks OK but my small concern is that the invalid case is handled
with a NumberFormatException here and it can be triggered by an
external request. I have not tested it here, but general notion is
that exception creation is expensive.
Maybe we can use a cached copy of NumberFormatException, like
o.a.naming.resources.ImmutableNameNotFoundException
By the way, there are two HexUtils classes in trunk. Do we need both?
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1132362 - in /tomcat/trunk/java/org/apache/coyote/http11:
AbstractHttp11Processor.java Http11AprProcessor.java Http11NioProcessor.java
Http11Processor.java
Posted by Mark Thomas <ma...@apache.org>.
On 05/06/2011 17:47, Rainer Jung wrote:
>>>> + for (int i = valueL - 1; i > colonPos; i--) {
>>>> + int charValue = HexUtils.getDec(valueB[i + valueS]);
>>>
>>> Any idea, why hex digits (including a-f, A-F) are allowed in port numbers?
>>>
>>> I know you only moved that code, but it reminded me of an observation I
>>> made long ago and forgot.
>>
>> I can't think of any good reason. Happy to limit that to the digits 0-9.
>
> Would you prefer renaming HexUtils to NumberUtils (or similar) and havin
> getDecFromHex() and getDecFromDec() there, or a new class similar to
> HexUtils called DecUtils?
Or we could just use Ascii.parseInt(char[], int, int)
Just testing a patch now...
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1132362 - in /tomcat/trunk/java/org/apache/coyote/http11:
AbstractHttp11Processor.java Http11AprProcessor.java Http11NioProcessor.java
Http11Processor.java
Posted by Rainer Jung <ra...@kippdata.de>.
>>> + for (int i = valueL - 1; i > colonPos; i--) {
>>> + int charValue = HexUtils.getDec(valueB[i + valueS]);
>>
>> Any idea, why hex digits (including a-f, A-F) are allowed in port numbers?
>>
>> I know you only moved that code, but it reminded me of an observation I
>> made long ago and forgot.
>
> I can't think of any good reason. Happy to limit that to the digits 0-9.
Would you prefer renaming HexUtils to NumberUtils (or similar) and havin
getDecFromHex() and getDecFromDec() there, or a new class similar to
HexUtils called DecUtils?
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1132362 - in /tomcat/trunk/java/org/apache/coyote/http11:
AbstractHttp11Processor.java Http11AprProcessor.java Http11NioProcessor.java
Http11Processor.java
Posted by Mark Thomas <ma...@apache.org>.
On 05/06/2011 14:09, Rainer Jung wrote:
> On 05.06.2011 12:06, markt@apache.org wrote:
>> Modified: tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
>> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1132362&r1=1132361&r2=1132362&view=diff
>> ==============================================================================
>> --- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original)
>> +++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Sun Jun 5 10:06:49 2011
>> @@ -41,6 +41,7 @@ import org.apache.juli.logging.Log;
>> import org.apache.tomcat.util.ExceptionUtils;
>> import org.apache.tomcat.util.buf.Ascii;
>> import org.apache.tomcat.util.buf.ByteChunk;
>> +import org.apache.tomcat.util.buf.HexUtils;
>> import org.apache.tomcat.util.buf.MessageBytes;
>> import org.apache.tomcat.util.http.FastHttpDateFormat;
>> import org.apache.tomcat.util.http.MimeHeaders;
>> @@ -967,6 +968,78 @@ public abstract class AbstractHttp11Proc
>>
>> abstract boolean prepareSendfile(OutputFilter[] outputFilters);
>>
>> + /**
>> + * Parse host.
>> + */
>> + protected void parseHost(MessageBytes valueMB) {
>> +
>> + if (valueMB == null || valueMB.isNull()) {
>> + // HTTP/1.0
>> + // If no host header, use the port info from the endpoint
>> + // The host will be obtained lazily from the socket if required
>> + // using ActionCode#REQ_LOCAL_NAME_ATTRIBUTE
>> + request.setServerPort(endpoint.getPort());
>> + return;
>> + }
>> +
>> + ByteChunk valueBC = valueMB.getByteChunk();
>> + byte[] valueB = valueBC.getBytes();
>> + int valueL = valueBC.getLength();
>> + int valueS = valueBC.getStart();
>> + int colonPos = -1;
>> + if (hostNameC.length < valueL) {
>> + hostNameC = new char[valueL];
>> + }
>> +
>> + boolean ipv6 = (valueB[valueS] == '[');
>> + boolean bracketClosed = false;
>> + for (int i = 0; i < valueL; i++) {
>> + char b = (char) valueB[i + valueS];
>> + hostNameC[i] = b;
>> + if (b == ']') {
>> + bracketClosed = true;
>> + } else if (b == ':') {
>> + if (!ipv6 || bracketClosed) {
>> + colonPos = i;
>> + break;
>> + }
>> + }
>> + }
>> +
>> + if (colonPos < 0) {
>> + if (!endpoint.isSSLEnabled()) {
>> + // 80 - Default HTTP port
>> + request.setServerPort(80);
>> + } else {
>> + // 443 - Default HTTPS port
>> + request.setServerPort(443);
>> + }
>> + request.serverName().setChars(hostNameC, 0, valueL);
>> + } else {
>> +
>> + request.serverName().setChars(hostNameC, 0, colonPos);
>> +
>> + int port = 0;
>> + int mult = 1;
>> + for (int i = valueL - 1; i > colonPos; i--) {
>> + int charValue = HexUtils.getDec(valueB[i + valueS]);
>
> Any idea, why hex digits (including a-f, A-F) are allowed in port numbers?
>
> I know you only moved that code, but it reminded me of an observation I
> made long ago and forgot.
I can't think of any good reason. Happy to limit that to the digits 0-9.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org