You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by Oleg Kalnichevski <ol...@apache.org> on 2005/12/06 21:01:02 UTC
Re: [HttpComponent][HttpCore] Refactoring of HTTP message
processing code
On Tue, 2005-12-06 at 09:07 -0500, Michael Becke wrote:
> Hi Oleg,
>
> As always your work on HttpComponents is quite impressive. I'm glad
> you've spent so much time making the API consistent, not only does
> this make it easier to use, but it should also be easier to maintain.
>
Thanks, Mike. Please do keep an eye on what I am doing to make sure I
will not drift away too much.
> > This problem can be solved by making Header an interface with two impls:
> > an immutable public class and a package private instantiated by passing
> > a reference to a CharArrayBuffer. I just thought that would be an
> > overkill though. Please let me know if you disagree
>
> Agreed, overkill.
>
> > * NumUtils#parseUnsignedInt is used instead of standard Integer#parseInt
> > to parse integer values in the HTTP messages such as protocol version
> > and chunk size. The parseUnsignedInt method produces no intermediate
> > garbage whereas the use of Integer#parseInt usually entails creation of
> > an intermediate string object
>
> Garbage reduction is also a worthwhile goal, though the integer
> parsing might be taking it a bit far. I think the performance tests
> should help decide this one.
I intend to run some benchmarks and see how the new components compare
to HttpClient 3.0 in terms of performance when running in different JREs
with different heap size. If we see no real benefit in reducing garbage
to an absolute minimum, I'll revert the code to use Integer#parseInt
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpclient-dev-help@jakarta.apache.org
Re: [HttpComponents] HttpClientConnection
Posted by Roland Weber <ht...@dubioso.net>.
Hi Oleg,
I had some more thoughts on the topic, especially since I overlooked the
RequestExecutor before. Handling expect/continue in the same location as
redirects and authentication was not a good idea from my side. Managing
connections with partially sent requests would introduce way too much
complexity to result in any kind of improvement. I was aiming at making
the connection interface a little more complicated while hiding that
from the user, but the changes I suggested would have to be propagated
through the RequestExecutor as well, where I don't want them any more
than you do. The connections are a little more intelligent than I had
expected them to be, I might as well get used to the idea.
Your suggestion to have a sendRequest that never returns a response and a
receiveRequest which can be called multiple times if the connection is
implemented or configured to return 1xx responses takes us a long way.
I am starting to think of expect/continue and 1xx responses as distinct
but linked problems. Discarding 1xx responses or giving applications
access to them is one thing, and blocking the sending of a request after
the header to wait for a 100 or terminal response is another.
I don't think that 1xx responses are the same as terminal responses, but
I consider them something that should not be completely hidden from an
application. That does not necessarily mean they have to be provided in
the same way as terminal responses. The cases in which access to a 1xx
response will be required are rare. I can image for example a callback
interface for delivering 1xx responses, while the terminal response is
returned as the result of a method invocation. I am sure we'll find a
good solution for this, preferably without different implementations
for the connection interface.
The blocking part of the expect/continue handshake is something I don't
feel comfortable with yet. Saying that, it is inherently synchronous and
I intend to simply ignore it for http-async, at least at the beginning.
I do have a tendency towards over-designing, but I will try to restrain
myself, and you guys can keep me in check if I fail in that :-)
cheers,
Roland
---------------------------------------------------------------------
To unsubscribe, e-mail: httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpclient-dev-help@jakarta.apache.org
Re: [HttpComponents] HttpClientConnection
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Wed, Dec 14, 2005 at 09:50:01PM +0100, Roland Weber wrote:
> Hi Oleg,
>
Hi Roland
See my comments in-line
>
> The general idea I have in mind is to isolate all knowledge about a
> sequence of request/reply/entity particles behind a single interface,
> which can be used for both synchronous and asynchronous operation.
> Here "knowledge about sequences" includes:
> - expect/continue handshake
> - redirect handling
> - authentication handling
> - ... (in case I forgot something :-)
I think this is essentially where we tend to differ. I believe that 1xx
status codes serve a different purpose compared to 3xx and 4xx and
therefore should be handled differently.
> It's a very long way to go, but we could then have different
> implementations of that interface for interactive user agents,
> proxies, non-interactive robots or spiders, and so on. That's
> why I would like to move the expect/continue handshake completely
> out of the connection itself.
Fine by me. I like this idea. However I personally find the interface
you have proposed too much of a price to be paid. What I dislike about
it is that it exposes mutable objects to the caller and leave it up to
the caller to assemble various bits and pieces into a consistent HTTP
message. I would very much prefer for all Http*Connection interfaces to
always consume and produce _consistent_ _immutable_ HTTP messages
leaving all the low level details to non-public impl classes
>
> > The HttpClientConnection interface no longer has any notion of a special
> > control logic such as the 'expect: continue' handshake. Just plain-vanilla
> > send a request, get a response stuff
> >
> > public interface HttpClientConnection extends HttpConnection {
> >
> > ...
> >
> > void sendRequest(HttpRequest request) throws HttpException, IOException;
> >
> > HttpResponse receiveResponse() throws HttpException, IOException;
> >
> > }
>
> This will not allow the control logic to be implemented outside of the
> connection interface. The control logic for expect/continue, if implemented
> outside of the connection, has to indicate "send the request header" versus
> "send the request entity". Your suggestion would require a specialized
> interface for this, or some hack like passing a flag in the parameters
> that are associated with the request. That's not really an improvement
> over the current situation.
Let me try to hack something up. I have a few ideas to try
>
> I guess our different lines of thought are due to the fact that the new API
> does not yet have a "sequencer" object like the method director. I would
> like to introduce one now for supporting expect/continue (to be extended for
> other uses later), while you would like to offer expect/continue without an
> additional abstraction layer and a new interface that has to be instantiated,
> right?
>
> How about this:
> - make the HttpClientConnection generic (following my initial proposal,
> but with MutableHttpRequest and MutableHttpEntity)
> - provide a static helper to handle the expect/continue handshake, which
> provides an interface similar to the current HttpClientConnection
>
>
See my objections / reservations described above
>
> Let me know what you think. I have a hunch we'll need some more
> iterations to come up with a sound concept.
>
I suggest we tackle the problem step by step in a series of incremental
patches. This approach worked well for us in the past when we had some
disagreements regarding the 3.0 API
I'll start working on a patch that will factor the existing control
logic out of the DefaultHttpClientConnection class. Once that is done we
can discuss various options how to change the public
HttpClientConnection.
Oleg
> cheers,
> Roland
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: httpclient-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: httpclient-dev-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpclient-dev-help@jakarta.apache.org
Re: [HttpComponents] HttpClientConnection
Posted by Roland Weber <ht...@dubioso.net>.
Hi Oleg,
thanks for your replies. See my comments inline.
I have quoted sections from both your mails into this one.
>> That results in inconsistent behavior: some 1xx responses are handled
>> by the connection itself, others are returned to the caller. The logic
>> to handle (or ignore) 1xx responses is spread across the code base.
>>
> I do not think this is the case. Only status codes 200 and above are
> ever returned to the caller. See
> DefaultHttpClientConnection#receiveResponse.
Whoops, my mistake. I should have taken the time to check that,
though it was not the main issue I have with the current design.
>>>public interface HttpClientConnection {
>>>
>>> public void sendRequest(HttpRequest req, HttpParams params)
>>>
>>> public void sendEntity(HttpEntity entity, HttpParams params)
>>>
>>> public HttpResponse receiveResponse(HttpParams params)
>>>
>>> public HttpEntity receiveEntity(HttpParams params)
>>>
>>>} // interface HttpClientConnection
>>>
>>>
>>
>>I think the idea to decouple request and entity processing logic as a
>>remedy for the problems identified above is a reasonable one. However, I
>>am not entirely sure it solves all the problems. At some point you will
>>have to assemble the complete HTTP response by combining the entity to
>>the header. However, since HttpResponse represents an immutable message
>>this will not be possible, unless you cast HttpResponse to
>>HttpMutableResponse, thus completely defeating the whole idea of
>>separating those two interfaces in the first place.
My mistake. As you suggested below, I should have used the MutableXxx
interfaces. I'm not yet familiar enough with the new API, but that will
get better over time :-)
The general idea I have in mind is to isolate all knowledge about a
sequence of request/reply/entity particles behind a single interface,
which can be used for both synchronous and asynchronous operation.
Here "knowledge about sequences" includes:
- expect/continue handshake
- redirect handling
- authentication handling
- ... (in case I forgot something :-)
It's a very long way to go, but we could then have different
implementations of that interface for interactive user agents,
proxies, non-interactive robots or spiders, and so on. That's
why I would like to move the expect/continue handshake completely
out of the connection itself.
>>Les us try to redesign the HttpClientConnection interface, however, I do
>>suspect strongly the only clean solution to the problem may be having
>>different interfaces representing different types of connections: a
>>regular client connection, a proxy-side connection and a client
>>connection capable of pipelining requests at the expense of not
>>supporting 1xx status codes. I am afraid we may fail to come up with a
>>reasonably clean interface capable of representing all different types
>>of connections.
I'd rather distinguish the usage scenario on a level on top of the
connections, as mentioned above.
> The HttpClientConnection interface no longer has any notion of a special
> control logic such as the 'expect: continue' handshake. Just plain-vanilla
> send a request, get a response stuff
>
> public interface HttpClientConnection extends HttpConnection {
>
> ...
>
> void sendRequest(HttpRequest request) throws HttpException, IOException;
>
> HttpResponse receiveResponse() throws HttpException, IOException;
>
> }
This will not allow the control logic to be implemented outside of the
connection interface. The control logic for expect/continue, if implemented
outside of the connection, has to indicate "send the request header" versus
"send the request entity". Your suggestion would require a specialized
interface for this, or some hack like passing a flag in the parameters
that are associated with the request. That's not really an improvement
over the current situation.
I guess our different lines of thought are due to the fact that the new API
does not yet have a "sequencer" object like the method director. I would
like to introduce one now for supporting expect/continue (to be extended for
other uses later), while you would like to offer expect/continue without an
additional abstraction layer and a new interface that has to be instantiated,
right?
How about this:
- make the HttpClientConnection generic (following my initial proposal,
but with MutableHttpRequest and MutableHttpEntity)
- provide a static helper to handle the expect/continue handshake, which
provides an interface similar to the current HttpClientConnection
/**
* Helper for straightforward request/response processing.
* This class implements handling of 1xx responses in general and
* the expect/continue handshake in particular.
*/
public final class ExpectContinueHelper {
// disabled default constructor
private ExpectContinueHelper() {
// no body
}
/**
* Sends a request, using expect/continue handshake if applicable.
* If expect/continue handshake is applicable and the server refuses
* to serve the request, this method will not send the request entity,
* but return the response from the server.
* Otherwise, the request and optional request entity get send
* and the response has to be received explicitly using
* {@link #receiveResponse receiveResponse}.
*
* @param conn the connection for sending and receiving
* @param request the request to send
*
* @return <code>null</code> if the request was sent without problems,
* or a final (non-1xx) response if the server refuses to handle
* the request during an expect/continue handshake
*
* @throws HttpException in case of a protocol problem
* @throws IOException in case of an IO exception
*/
public final static HttpResponse sendRequest(
HttpClientConnection conn,
HttpRequest request
) throws HttpException, IOException {
...
}
/**
* Receives a response.
* This method receives a response after
* {@link #sendRequest sendRequest}
* has been called and returned <code>null</code>.
*
* @param conn the connection for receiving
* @param request the request for which to receive the response.
* It <i>must</i> have been sent before using
* {@link #sendRequest sendRequest} over the same
* connection, with <code>null</code>
* as the return value!
*
* @return the final (non-1xx) response to the request
*
* @throws HttpException in case of a protocol problem
* @throws IOException in case of an IO exception
*/
public final static HttpResponse receiveResponse(
HttpClientConnection conn,
HttpRequest request
) throws HttpException, IOException {
...
}
} // class ExpectContinueHelper
This would allow for moving the expect/continue logic outside of the
client connection, while leaving the complexity of the ElementalHttpGet
and ElementalHttpPost examples basically unchanged, and getting people
used to the fact the HTTP is just a little too complex to use an
HttpClientConnection directly.
Of course it would totally screw up the current usage model for the
HttpRequestExceutor, though I could live with the executor being
hard-wired against the ExpectContinueHelper for the time being.
And the name of the helper class is not intuitive, because it has to be
called for all requests and not just for those that will or might use
an expect/continue handshake.
Let me know what you think. I have a hunch we'll need some more
iterations to come up with a sound concept.
cheers,
Roland
---------------------------------------------------------------------
To unsubscribe, e-mail: httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpclient-dev-help@jakarta.apache.org
Re: [HttpComponents] HttpClientConnection
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Sun, Dec 11, 2005 at 10:25:34PM +0100, Oleg Kalnichevski wrote:
> On Sun, 2005-12-11 at 15:24 +0100, Roland Weber wrote:
<snip/>
> >
> > public interface HttpClientConnection {
> >
> > public void sendRequest(HttpRequest req, HttpParams params)
> >
> > public void sendEntity(HttpEntity entity, HttpParams params)
> >
> > public HttpResponse receiveResponse(HttpParams params)
> >
> > public HttpEntity receiveEntity(HttpParams params)
> >
> > } // interface HttpClientConnection
> >
> >
>
> I think the idea to decouple request and entity processing logic as a
> remedy for the problems identified above is a reasonable one. However, I
> am not entirely sure it solves all the problems. At some point you will
> have to assemble the complete HTTP response by combining the entity to
> the header. However, since HttpResponse represents an immutable message
> this will not be possible, unless you cast HttpResponse to
> HttpMutableResponse, thus completely defeating the whole idea of
> separating those two interfaces in the first place.
>
> Les us try to redesign the HttpClientConnection interface, however, I do
> suspect strongly the only clean solution to the problem may be having
> different interfaces representing different types of connections: a
> regular client connection, a proxy-side connection and a client
> connection capable of pipelining requests at the expense of not
> supporting 1xx status codes. I am afraid we may fail to come up with a
> reasonably clean interface capable of representing all different types
> of connections.
>
> Oleg
>
> >
> > Passing 'params' to sendRequest() is not necessary since params are
> > available from HttpRequest, but it is more consistent with the
> > receive methods that need the params argument. sendRequest() sends
> > only the request line, headers, and empty line, but no entity.
> > sendEntity() could take an HttpEntityEnclosingRequest instead of the
> > entity itself as an argument, though it shouldn't need anything but
> > the entity and params.
> > receiveResponse() receives only the status line and request headers.
> > It doesn't need the request anymore, since it doesn't have to decide
> > whether the response includes an entity. It returns all responses,
> > including those with 1xx status codes. receiveEntity() could take
> > an EntityGenerator as an argument, in that case sendEntity() should
> > be changed to take an EntityWriter for the sake of symmetry.
> > I have not dug deeper into the code, so I don't know how the data
> > transmitter below the connection is affected by these suggestions.
> >
> >
> > I'll be on a business trip for a few days, but I'm looking forward
> > to reading your comments and responding when I'm back.
> >
> > cheers,
> > Roland
Roland,
I thought the whole problem over a little more. The idea to factor out 1xx
handling logic from the low level connection class does make a lot of sense
conceptually. However, I do not think that the new interface makes a better
public API than the existing one
I suggest we factor out the common client connection code into an abstract
impl class. The class will be state-less and will have no special knowledge
of the 1xx status codes. The class will have a similar method signature to
that you have proposed.
class org.apache.http.impl.AbstractSendingHttpConnection extends org.apache.http.impl.AbstractHttpConnection {
protected void sendRequestHeader(HttpRequest req);
protected void sendRequestEntity(HttpEntityEnclosingRequest req)
protected HttpMutableResponse receiveResponseHeader(HttpParams params);
protected HttpMutableEntity sendRequestEntity(HttpParams params);
}
The HttpClientConnection interface no longer has any notion of a special
control logic such as the 'expect: continue' handshake. Just plain-vanilla
send a request, get a response stuff
public interface HttpClientConnection extends HttpConnection {
...
void sendRequest(HttpRequest request) throws HttpException, IOException;
HttpResponse receiveResponse() throws HttpException, IOException;
}
The DefaultHttpClientConnection subclasses the new
AbstractSendingHttpConnection class and is made the 'expect: continue'
handshake aware. The control logic can be implemented internally (this
will require the class to maintain the state rendering it stateful and
threading-unsafe) or externally in the RequestExecutor (this will
render the class modal. The RequestExecutor will have to know the mode
(the sequence of method calls) required to produce the desired effect)
public class DefaultHttpClientConnection implements HttpClientConnection extends AbstractSendingHttpConnection {
}
The DefaultAsyncHttpClientConnection subclasses the new
AbstractSendingHttpConnection class and is made capable of 'pipelining'
requests at the expense of not supporting the 1xx based control logic
public class DefaultAsyncHttpClientConnection implements HttpClientConnection extends AbstractSendingHttpConnection {
}
Can you live with that?
Oleg
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: httpclient-dev-unsubscribe@jakarta.apache.org
> > For additional commands, e-mail: httpclient-dev-help@jakarta.apache.org
> >
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: httpclient-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: httpclient-dev-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpclient-dev-help@jakarta.apache.org
Re: [HttpComponent] HttpClientConnection
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Sun, 2005-12-11 at 15:24 +0100, Roland Weber wrote:
> Hi Oleg,
>
Hi Roland,
Please see my comments in-line
> I have taken a look at the HttpClientConnection and it's default
> implementation. You have implemented some protocol logic on the
> connection level which I would have expected to find in the method
> director. This design has serious drawbacks. Let me first explain
> the problems I see, then suggest an alternative interface.
>
>
> 1. HttpClientConnection.sendRequest will check for expect-continue
> handshake and wait for a 100-continue response if that is enabled.
> This clashes with asynchronous communication, in particular with
> pipelining. Several other requests may previously have been sent
> to the server, for which the responses have not yet been received.
> The connection will race with some other thread responsible for
> receiving responses to the preceeding requests.
>
Agreed.
>
> 2. HttpClientConnection.sendRequest will handle exactly one 1xx
> response if and only if the expect-continue handshake is enabled.
> RFC 2616, section 10.1 states:
>
> > A client MUST be prepared to accept one or more 1xx status responses
> > prior to a regular response, even if the client does not expect a 100
> > (Continue) status message.
>
> That results in inconsistent behavior: some 1xx responses are handled
> by the connection itself, others are returned to the caller. The logic
> to handle (or ignore) 1xx responses is spread across the code base.
>
I do not think this is the case. Only status codes 200 and above are
ever returned to the caller. See
DefaultHttpClientConnection#receiveResponse. I do have to admit that due
to the fact that the DefaultHttpClientConnection attempts to handle 1xx
status code internally it cannot be used for as a basis for implementing
HTTP/1.1 compliant proxies
>
> 3. HttpClientConnection.sendRequest will not give access to the
> first 1xx response if expect-continue handshake is enabled.
> RFC 2616, section 10.1 states:
>
> > Proxies MUST forward 1xx responses, unless the connection between the
> > proxy and its client has been closed, or unless the proxy itself
> > requested the generation of the 1xx response.
>
> Consider the following scenario for a proxy that uses HttpComponents
> to connect to the server. Note that this is *not* an academic scenario:
>
> - Client sends request with expect-continue handshake to the proxy.
> - Proxy MUST forward request with expect-continue handshake to server
> if server is HTTP/1.1 compliant (RFC 2616, section 8.2.3)
> - Server responds with 100-continue, waits for the request body.
> - Proxy MUST forward the 100-continue response to the client (see above).
> This is not possible, since the response is handled by the connection
> and never seen by the proxy code using the connection.
> - Proxy/connection can not forward the request body, since the client
> is still waiting for a 100-continue response before sending it.
> - Client doesn't receive a 100-continue response from proxy, remains
> stalled until some timeout hits (suggested in 8.2.3), then either
> aborts the request altogether or finally transmits the response body
> after an unnecessarily long delay.
>
>
Agreed.
> 4. (minor:) HttpClientConnection.receiveResponse requires the request
> as an argument, although the request has already been sent. This is
> necessary because receiveResponse has to decide whether the response
> has a body, which depends on the request method (HEAD vs. GET). Apart
> from that, only the parameters are accessed through the request. It
> is not intuitive that the request has to be passed again after it has
> been sent.
>
>
Agreed.
>
> Because of these problems, I suggest to completely remove the protocol
> logic from the HttpClientConnection interface. In particular, knowledge
> about the sequence of requests, responses, and entities, should be
> implemented exclusively on a layer above the connection. Also, sending
> and receiving should be completely decoupled, to allow for asynchronous
> communication. These are the methods I propose instead of the current
> sendRequest and receiveResponse:
>
> public interface HttpClientConnection {
>
> public void sendRequest(HttpRequest req, HttpParams params)
>
> public void sendEntity(HttpEntity entity, HttpParams params)
>
> public HttpResponse receiveResponse(HttpParams params)
>
> public HttpEntity receiveEntity(HttpParams params)
>
> } // interface HttpClientConnection
>
>
I think the idea to decouple request and entity processing logic as a
remedy for the problems identified above is a reasonable one. However, I
am not entirely sure it solves all the problems. At some point you will
have to assemble the complete HTTP response by combining the entity to
the header. However, since HttpResponse represents an immutable message
this will not be possible, unless you cast HttpResponse to
HttpMutableResponse, thus completely defeating the whole idea of
separating those two interfaces in the first place.
Les us try to redesign the HttpClientConnection interface, however, I do
suspect strongly the only clean solution to the problem may be having
different interfaces representing different types of connections: a
regular client connection, a proxy-side connection and a client
connection capable of pipelining requests at the expense of not
supporting 1xx status codes. I am afraid we may fail to come up with a
reasonably clean interface capable of representing all different types
of connections.
Oleg
>
> Passing 'params' to sendRequest() is not necessary since params are
> available from HttpRequest, but it is more consistent with the
> receive methods that need the params argument. sendRequest() sends
> only the request line, headers, and empty line, but no entity.
> sendEntity() could take an HttpEntityEnclosingRequest instead of the
> entity itself as an argument, though it shouldn't need anything but
> the entity and params.
> receiveResponse() receives only the status line and request headers.
> It doesn't need the request anymore, since it doesn't have to decide
> whether the response includes an entity. It returns all responses,
> including those with 1xx status codes. receiveEntity() could take
> an EntityGenerator as an argument, in that case sendEntity() should
> be changed to take an EntityWriter for the sake of symmetry.
> I have not dug deeper into the code, so I don't know how the data
> transmitter below the connection is affected by these suggestions.
>
>
> I'll be on a business trip for a few days, but I'm looking forward
> to reading your comments and responding when I'm back.
>
> cheers,
> Roland
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: httpclient-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: httpclient-dev-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpclient-dev-help@jakarta.apache.org
[HttpComponent] HttpClientConnection
Posted by Roland Weber <ht...@dubioso.net>.
Hi Oleg,
I have taken a look at the HttpClientConnection and it's default
implementation. You have implemented some protocol logic on the
connection level which I would have expected to find in the method
director. This design has serious drawbacks. Let me first explain
the problems I see, then suggest an alternative interface.
1. HttpClientConnection.sendRequest will check for expect-continue
handshake and wait for a 100-continue response if that is enabled.
This clashes with asynchronous communication, in particular with
pipelining. Several other requests may previously have been sent
to the server, for which the responses have not yet been received.
The connection will race with some other thread responsible for
receiving responses to the preceeding requests.
2. HttpClientConnection.sendRequest will handle exactly one 1xx
response if and only if the expect-continue handshake is enabled.
RFC 2616, section 10.1 states:
> A client MUST be prepared to accept one or more 1xx status responses
> prior to a regular response, even if the client does not expect a 100
> (Continue) status message.
That results in inconsistent behavior: some 1xx responses are handled
by the connection itself, others are returned to the caller. The logic
to handle (or ignore) 1xx responses is spread across the code base.
3. HttpClientConnection.sendRequest will not give access to the
first 1xx response if expect-continue handshake is enabled.
RFC 2616, section 10.1 states:
> Proxies MUST forward 1xx responses, unless the connection between the
> proxy and its client has been closed, or unless the proxy itself
> requested the generation of the 1xx response.
Consider the following scenario for a proxy that uses HttpComponents
to connect to the server. Note that this is *not* an academic scenario:
- Client sends request with expect-continue handshake to the proxy.
- Proxy MUST forward request with expect-continue handshake to server
if server is HTTP/1.1 compliant (RFC 2616, section 8.2.3)
- Server responds with 100-continue, waits for the request body.
- Proxy MUST forward the 100-continue response to the client (see above).
This is not possible, since the response is handled by the connection
and never seen by the proxy code using the connection.
- Proxy/connection can not forward the request body, since the client
is still waiting for a 100-continue response before sending it.
- Client doesn't receive a 100-continue response from proxy, remains
stalled until some timeout hits (suggested in 8.2.3), then either
aborts the request altogether or finally transmits the response body
after an unnecessarily long delay.
4. (minor:) HttpClientConnection.receiveResponse requires the request
as an argument, although the request has already been sent. This is
necessary because receiveResponse has to decide whether the response
has a body, which depends on the request method (HEAD vs. GET). Apart
from that, only the parameters are accessed through the request. It
is not intuitive that the request has to be passed again after it has
been sent.
Because of these problems, I suggest to completely remove the protocol
logic from the HttpClientConnection interface. In particular, knowledge
about the sequence of requests, responses, and entities, should be
implemented exclusively on a layer above the connection. Also, sending
and receiving should be completely decoupled, to allow for asynchronous
communication. These are the methods I propose instead of the current
sendRequest and receiveResponse:
public interface HttpClientConnection {
public void sendRequest(HttpRequest req, HttpParams params)
public void sendEntity(HttpEntity entity, HttpParams params)
public HttpResponse receiveResponse(HttpParams params)
public HttpEntity receiveEntity(HttpParams params)
} // interface HttpClientConnection
Passing 'params' to sendRequest() is not necessary since params are
available from HttpRequest, but it is more consistent with the
receive methods that need the params argument. sendRequest() sends
only the request line, headers, and empty line, but no entity.
sendEntity() could take an HttpEntityEnclosingRequest instead of the
entity itself as an argument, though it shouldn't need anything but
the entity and params.
receiveResponse() receives only the status line and request headers.
It doesn't need the request anymore, since it doesn't have to decide
whether the response includes an entity. It returns all responses,
including those with 1xx status codes. receiveEntity() could take
an EntityGenerator as an argument, in that case sendEntity() should
be changed to take an EntityWriter for the sake of symmetry.
I have not dug deeper into the code, so I don't know how the data
transmitter below the connection is affected by these suggestions.
I'll be on a business trip for a few days, but I'm looking forward
to reading your comments and responding when I'm back.
cheers,
Roland
---------------------------------------------------------------------
To unsubscribe, e-mail: httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpclient-dev-help@jakarta.apache.org