hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oleg Kalnichevski <ol...@apache.org>
Subject Re: [HttpCore] HttpCore 4.1-beta1 release preview
Date Tue, 23 Mar 2010 19:41:50 GMT
James Leigh wrote:
> On Mon, 2010-03-22 at 23:20 +0100, Oleg Kalnichevski wrote:
>> On Mon, 2010-03-22 at 17:50 -0400, James Leigh wrote:
>>> On Sun, 2010-03-21 at 14:36 +0100, Oleg Kalnichevski wrote:
>>>> Folks
>>>>
>>>> Please DO try to find a few minutes to review the release notes and 
>>>> release packages for the coming HttpCore 4.1-beta1
>>>>
>>>> Release notes:
>>>> http://people.apache.org/~olegk/httpcore-4.1-beta1-preview/RELEASE_NOTES.txt
>>>>
>>>> Release packages:
>>>> http://people.apache.org/~olegk/httpcore-4.1-beta1-preview/
>>>>
>>>> If I hear no complaints I will proceed with building the official 
>>>> release packages in a few days.
>>>>
>>>> Oleg
>>>>
>>> Oleg, can you check on LengthDelimitedDecoder logic for Content-Length:
>>> 0? Looking at the code below (from the preview release) I don't think it
>>> will ever change the status to complete.
>>>
>> I am too tired to think clearly right now. I quickly put together a test
>> case for zero length content and seems to work just fine:
>>
>> http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/impl/nio/codecs/TestLengthDelimitedDecoder.java?r1=926372&r2=926371&pathrev=926372
>>
>> Please take a look at the unit tests for the LengthDelimitedDecoder.
>> HttpCore NIO codecs are well unit testable using mock channels. Feel
>> free to add more tests for this condition. If you still think there is a
>> problem, please go ahead and raise a JIRA for the issue. I'll look at it
>> tomorrow.
>>
>>
>>> On the line 95 dst.limit(newLimit), the limit is set to zero, so the
>>> next read operation returns zero. This causes a perpetual read of zero
>>> bytes and never completes. Perhaps it should have another condition to
>>> complete immediately if the content length is zero?
>>>
>> There is no loop in the #read method, so the codec cannot enter an
>> infinite loop. Even if no data has been read from the channel, the
>> following condition should always resolve to true when
>> this.contentLength is zero and the codec state will immediately get
>> changed to completed.
>>
>> if (this.len >= this.contentLength) {
>>   this.completed = true;
>> }
>>
>> All seems well.
>>
>> Cheers
>>
>> Oleg
>>
> 
> Ah yes, it is okay. I think I was expecting the decoder to already be
> complete or to at least return -1 on the first read. No need to create a
> bug report as it is functioning as expected.
> 
> Just so I understand this. In order to detect if a decoder has nothing
> to read you have to use the following condition (I guess that is how
> previous versions worked)?
> 
> if (decoder.isCompleted()
> 	|| decoder.read(ByteBuffer.allocate(0)) < 0
> 	|| decoder.isCompleted()) {
>     // nothing to read
> }
> 

I would say decoder.isCompleted() alone should be enough. If the end of 
stream condition (-1) is detected all decoders should set their state to 
completed automatically.

Cheers

Oleg


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


Mime
View raw message