Re: HttpClient bug? and Bug 5018 PATCH

[Rodney Waldhoff <rw...@hotmail.com>]

Sorry guys, I've been swamped at that pesky day job.

I'll try to get to both of these this weekend if no one beats me to it.

_________________________________________________________________
Get your FREE download of MSN Explorer at http://explorer.msn.com/intl.asp


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

Re: HttpClient bug? and Bug 5018 PATCH

["Paul C. Bryan" <pb...@creatia.com>]

Rodney Waldhoff wrote:

> Sorry guys, I've been swamped at that pesky day job.


I feel your pain.

> I'll try to get to both of these this weekend if no one beats me to it.


I've attached a new patch, with a minor change to handle case where 
chunk-extension(s) are included in the chunk size line. I've also 
updated my comments below to be more accurate.

----- BEGIN PATCH COMMENTS -----

SUMMARY:

Chunked transfer encoding not isolated from application.

DESCRIPTION:

Chunked transfer encoding is not being supported transparently by the 
HttpMethodBase object, causing chunk data to be embedded in response 
body data and forcing the application to handle the HTTP/1.1 
implementation of chunked transfer encoding.

The included patch now properly parses chunk data as per RFC 2616 and
provides body content consistently, regardless of whether chunked 
transfer encoding was used by the server or not. This relieves the 
application from the requirement of implementing HTTP/1.1.

PATCH NOTES:

RFC 2616 specifies optional footer fields that are supplied after the 
end of the chunked data body. These are parsed, but added as "header" 
fields because I didn't want to change the HttpMethod interface. The 
result is that footer fields such as signatures must be retrieved as 
headers. The downside to this approach is there is no way to distinguish 
between a header field and a footer field when interrogating the HttpMethod.

I've defined a constant SIZE_buf to use as a buffer size for allocated 
copy buffers. I've only used it in my patch (because it is referred to 
more than once in the method), but propose that it be used by any 
methods that perform copy operations with the allocation of a buffer. 
Comments?

Because a ResponseInputStream is used to read the body, the response 
footers cannot be retrived using the conn.readLine() method as the 
headers were retrieved. Instead, we must read lines from 
ResponseInputStream must be read. For this reason, response
footers are read after the body in the readResponseBody method.

I'm not sure why only get request method is supporting writing content 
to disk. I believe the useDisk functionality should be moved to 
HttpMethodBase. I was tempted to do so, but didn't want to jeopardize 
the acceptance of this patch. If someone gives me the go ahead, I am 
willing to move useDisk into HttpMethodBase.

GetMethod.readResponseBody(HttpState, HttpConnection) duplicated much of 
HttpMethodBase.readResponseBody(HttpState, HttpConnection) 
functionality. I moved this into a new common 
readResponseBody(ResponseInputStream, OutputStream) that is shared by 
HttpMethodBase and GetMethod. If useDisk moves into HttpMethodBase, 
there should be no longer be a need for this separate method.

----- END PATCH COMMENTS -----

Yours truly,

Paul C. Bryan <pb...@creatia.com>
http://pbryan.net/