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 Mon, 22 Mar 2010 22:20:17 GMT
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-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
> 



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


Mime
View raw message