You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by Roland Weber <ht...@dubioso.net> on 2006/02/04 17:40:05 UTC

Re: [Bug 38509] - entities and connection handling incomplete

Hi Oleg, hi all,

I've noticed some peculiarities in the HttpEntity interface.

First of all, the JavaDocs for getContent() explicitly state
that it is a programming error to return the same InputStream
for multiple invocations. In http-async, I used
    entity.getContent().close();
to make sure that the response entity is read to the end before
the connection is re-used. That was a programming error on my part.
What's worse, it worked: BasicHttpEntity does return the same
InputStream on each call, and so does GzipDecompressingEntity.

One way to solve this is to change the JavaDocs so that only
repeatable entities are required to return a new InputStream
on each invocation. Another way to solve this is to change the
non-repeatable entities to return their one and only stream
only for the first invocation, and null for the following ones.

I prefer the second option. No matter what we do, somebody is
going to use entities the wrong way. But if null is returned,
they get an exception and can fix their code quickly. If some
entities return the same stream and others a new one, then the
problems will be very hard to debug.


The second item is the return value of writeTo(). Oleg, did
you have a particular use case in mind when you defined that?
If 'false' is returned, then some other thread must be busy
writing the entity asynchronously. But I'm not aware of means
to synchronize with that thread's operation. For example, the
default implementations of the client connection and entity
writer will, after a call to conn.sendRequestEntity(), simply
return while some thread somewhere is still writing to the
connection.
This feature feels like a big can of worms to me. If there is
no specific use case, we should rather require writeTo() to
write the entity completely in all cases.


And finally, I had some problems to write JavaDocs for isChunked.
The meaning of the flag is different for outgoing and incoming
entities. I completely failed to come up with a recommendation
for wrapping entities. Please review the JavaDocs I've written.


cheers,
  Roland

---------------------------------------------------------------------
To unsubscribe, e-mail: httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpclient-dev-help@jakarta.apache.org


Re: [Bug 38509] - entities and connection handling incomplete

Posted by Roland Weber <ht...@dubioso.net>.
Hi Oleg,

>>I would rather prefer an explicit IllegalStateException when
>>getContent() is called on an entity whose content has already been
>>consumed.

I've implemented that. An IllegalStateException is thrown when the
content of BasicHttpEntity is obtained several times, or if it is
obtained without being set at all. Now I have 12 failing test cases.
This will take a while to clean up.

cheers,
  Roland

---------------------------------------------------------------------
To unsubscribe, e-mail: httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpclient-dev-help@jakarta.apache.org


Re: [Bug 38509] - entities and connection handling incomplete

Posted by Roland Weber <ht...@dubioso.net>.
Hi Oleg,

> I would rather prefer an explicit IllegalStateException when
> getContent() is called on an entity whose content has already been
> consumed.

Yes, that's better.

>>The second item is the return value of writeTo(). Oleg, did
>>you have a particular use case in mind when you defined that?
> 
> I am fine to revert the change 

Ok. I'll make this my test case for creating a patch that I can
apply with "diff". I can't promise a result for tomorrow though.
It's the Snooker final and they're broadcasting both sessions :-)

cheers,
  Roland

---------------------------------------------------------------------
To unsubscribe, e-mail: httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpclient-dev-help@jakarta.apache.org


Re: [Bug 38509] - entities and connection handling incomplete

Posted by Oleg Kalnichevski <ol...@apache.org>.
On Sat, 2006-02-04 at 17:40 +0100, Roland Weber wrote:
> Hi Oleg, hi all,
> 
> I've noticed some peculiarities in the HttpEntity interface.
> 
> First of all, the JavaDocs for getContent() explicitly state
> that it is a programming error to return the same InputStream
> for multiple invocations. In http-async, I used
>     entity.getContent().close();
> to make sure that the response entity is read to the end before
> the connection is re-used. That was a programming error on my part.
> What's worse, it worked: BasicHttpEntity does return the same
> InputStream on each call, and so does GzipDecompressingEntity.
> 
> One way to solve this is to change the JavaDocs so that only
> repeatable entities are required to return a new InputStream
> on each invocation. Another way to solve this is to change the
> non-repeatable entities to return their one and only stream
> only for the first invocation, and null for the following ones.
>
> I prefer the second option. No matter what we do, somebody is
> going to use entities the wrong way. But if null is returned,
> they get an exception and can fix their code quickly. If some
> entities return the same stream and others a new one, then the
> problems will be very hard to debug.
> 

I would rather prefer an explicit IllegalStateException when
getContent() is called on an entity whose content has already been
consumed.

> 
> The second item is the return value of writeTo(). Oleg, did
> you have a particular use case in mind when you defined that?

Yes, I did. I introduced this condition, while working on a Tomcat
Coyote connector implementation based on HttpCore. Think of Servlet API,
which does not have a concept of an entity as such. In a servlet one
simply uses an OutputStream or a Writer to stream out the content after
the HTTP response header has been committed. Initially I used a
bastardized HttpEntity to keep a reference to the underlying
OutputStream instance. Later on I completely gave up on the idea of
using the HttpEntity interface in the context of a servlet engine and
simply implemented a Coyote specific HTTP connection class capable of
stream content directly.

I admit this idea was ill conceived. I am fine to revert the change 

> If 'false' is returned, then some other thread must be busy
> writing the entity asynchronously. But I'm not aware of means
> to synchronize with that thread's operation. For example, the
> default implementations of the client connection and entity
> writer will, after a call to conn.sendRequestEntity(), simply
> return while some thread somewhere is still writing to the
> connection.
> This feature feels like a big can of worms to me. If there is
> no specific use case, we should rather require writeTo() to
> write the entity completely in all cases.
> 
> 
> And finally, I had some problems to write JavaDocs for isChunked.
> The meaning of the flag is different for outgoing and incoming
> entities. I completely failed to come up with a recommendation
> for wrapping entities. Please review the JavaDocs I've written.
> 

Looks good to me.

Cheers,

Oleg


---------------------------------------------------------------------
To unsubscribe, e-mail: httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpclient-dev-help@jakarta.apache.org