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-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org

Reply via email to