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 > Normally, I would create a bug for this, but because of the timing I > wanted to bring this up for discussion right away. > > -- > Thanks, > James > > public class LengthDelimitedDecoder extends AbstractContentDecoder > implements FileContentDecoder { > > private final long contentLength; > > private long len; > > public LengthDelimitedDecoder( > final ReadableByteChannel channel, > final SessionInputBuffer buffer, > final HttpTransportMetricsImpl metrics, > long contentLength) { > super(channel, buffer, metrics); > if (contentLength < 0) { > throw new IllegalArgumentException("Content length may not be > negative"); > } > this.contentLength = contentLength; > } > > public int read(final ByteBuffer dst) throws IOException { > if (dst == null) { > throw new IllegalArgumentException("Byte buffer may not be null"); > } > if (this.completed) { > return -1; > } > int lenRemaining = (int) (this.contentLength - this.len); > > int bytesRead; > if (this.buffer.hasData()) { > int maxLen = Math.min(lenRemaining, this.buffer.length()); > bytesRead = this.buffer.read(dst, maxLen); > } else { > if (dst.remaining() > lenRemaining) { > int oldLimit = dst.limit(); > int newLimit = oldLimit - (dst.remaining() - lenRemaining); > dst.limit(newLimit); > bytesRead = this.channel.read(dst); > dst.limit(oldLimit); > } else { > bytesRead = this.channel.read(dst); > } > if (bytesRead > 0) { > this.metrics.incrementBytesTransferred(bytesRead); > } > } > if (bytesRead == -1) { > this.completed = true; > return -1; > } > this.len += bytesRead; > if (this.len >= this.contentLength) { > this.completed = true; > } > return bytesRead; > } > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org > For additional commands, e-mail: dev-h...@hc.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org For additional commands, e-mail: dev-h...@hc.apache.org
