commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ryan Lubke <Ryan.Lu...@Sun.COM>
Subject Re: [Fwd: [HttpClient][PATCH] Correction to addHostRequestHeader +minor formatting cleanups]
Date Thu, 11 Jul 2002 12:51:21 GMT
On Wed, 2002-07-10 at 22:52, Jeff Dever wrote:
> -1 I'd suggest that Ryan re-work a couple things and submit another
> patch.
> 
> rcf2616:
>    A client MUST include a Host header field in all HTTP/1.1 request
>    messages . If the requested URI does not include an Internet host
>    name for the service being requested, then the Host header field MUST
>    be given with an empty value.
> 
> So you are saying that because an ipaddress is not an internet host
> name, the header field must be present but with an empty value.  Seems
> like a reasonable intreptation.  Can anyone confirm that?
> 
> While you are in the addHostRequestHeader, you might want to take care
> of that little unlabled todo comment.  The host header should only be
> sent when using HTTP/1.1 and be completely left out for HTTP/1.0.  You
> can search rfc1945 and rfc2616 for "Request Header Fields" to confirm. 
> You can use the http11 boolean variable in a conditional.

Actually, I think section 19.6.1.1 of RFC 2616 covers this so that the
Host header doesn't have to be conditionally sent.


> 
> 
> For your isIpAddress method, it would have been nice if sun gave us a
> InetAddress.isIpAddress static method or somthing.  But it doesn't see
> to be convinent to use the InetAddress class.  Too bad.

> 
> - You may wnat to trim() your input string, else a leading/trailing
> blank will cause it to return false.

Good point.
> 
> - I think a input value string of 259 sequential "." characters will
> return true.

It's possible, however, realistically, would this ever be provided as a
target host :)

> 
> - You might be better off using a StringTokenizer and split on the dot. 
> you can then call countTokens() to see if it is in the ball park and
> then then use Integer.parseInt on the tokens.  Return false immeadiately
> when you find a non-conformity.  Still would prefer using some Java.net
> library method though, but I can't see anything there that would be
> useful.

I originally was doing something similar to this, but using a char[]
would yield better performance as this would be with each request.

Like the comment states, it's a quick and dirty check and provides
little in the way of validation aside from being numeric and a
quad-address.  I'll clean it up and resubmit.  

Thanks for the input.

> 
> - You may also wish to add a test case to TestMethodsNoHost for your
> isIpAddress method.

Will do.
> 
> 
> 
> Ryan Lubke wrote:
> > 
> > Hello,
> > 
> > I've attached a patch to correct the addHostRequestHeader() method.
> > 
> > The method, as it currently stands, doesn't take into account the fact
> > that the request may be directed at a port other than 80.
> > 
> > Additionally, if the value for the target host is an IP address, the
> > current logic will sent that value across in the Host header, where as,
> > the best I can tell, the 2616 states that if the host value is not
> > an Internet Hostname, then the value for the Host header should be
> > empty.
> > 
> > So the areas of note in the patch are changes to addHostRequestHeader(),
> > and a new private method, isIpAddress(String value).
> > 
> > NOTE:  The check performed on the value could be considered weak.  All
> > it cares about is that there are 4 all-digit values separated by 3 '.'.
> > If there is a better way to do this, please let me know.
> > 
> > At any rate, I've tested this out and it seems to work OK.
> > 
> > -rl
> > 
> >   ------------------------------------------------------------------------
> >                                 Name: HttpMethodBase.java.patch
> >    HttpMethodBase.java.patch    Type: text/x-patch
> >                             Encoding: quoted-printable
> > 
> >   ------------------------------------------------------------------------
> > --
> > To unsubscribe, e-mail:   <mailto:commons-dev-unsubscribe@jakarta.apache.org>
> > For additional commands, e-mail: <mailto:commons-dev-help@jakarta.apache.org>
> 
> --
> To unsubscribe, e-mail:   <mailto:commons-dev-unsubscribe@jakarta.apache.org>
> For additional commands, e-mail: <mailto:commons-dev-help@jakarta.apache.org>
> 



--
To unsubscribe, e-mail:   <mailto:commons-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:commons-dev-help@jakarta.apache.org>


Mime
View raw message