Mailing-List: contact httpclient-dev-help@jakarta.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "HttpClient Project" <httpclient-dev@jakarta.apache.org>
Received-SPF: neutral (asf.osuosl.org: local policy)
Subject: Re:  [Bug 38509]  - entities and connection handling incomplete
From: Oleg Kalnichevski <olegk@apache.org>
To: HttpClient Project <httpclient-dev@jakarta.apache.org>
In-Reply-To: <43E4D8E5.9030400@dubioso.net>
References: <20060204161102.32295CB@ajax.apache.org>
	 <43E4D8E5.9030400@dubioso.net>
Content-Type: text/plain
Date: Sat, 04 Feb 2006 19:20:26 +0100
Message-Id: <1139077226.27460.25.camel@localhost.localdomain>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit

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