You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by Oleg Kalnichevski <ol...@apache.org> on 2010/03/21 14:36:51 UTC
[HttpCore] HttpCore 4.1-beta1 release preview
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [HttpCore] HttpCore 4.1-beta1 release preview
Posted by Oleg Kalnichevski <ol...@apache.org>.
Asankha C. Perera wrote:
> Hi Oleg
>
> Minor comment on the artifact names, since it still states alpha2 - we
> can fix it when you do the real RC.
>
> i.e.
> http://people.apache.org/~olegk/httpcore-4.1-beta1-preview/httpcomponents-core-4.1-alpha2-SNAPSHOT-bin.zip
>
> I hope to check the actual artifacts and get back with any comments by
> around Monday
>
> cheers
> asankha
>
Hi Asankha
Just disregard the artifact names. I intentionally did not tag the
release as yet until some basic checks are done.
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [HttpCore] HttpCore 4.1-beta1 release preview
Posted by "Asankha C. Perera" <as...@apache.org>.
Hi Oleg
Minor comment on the artifact names, since it still states alpha2 - we
can fix it when you do the real RC.
i.e.
http://people.apache.org/~olegk/httpcore-4.1-beta1-preview/httpcomponents-core-4.1-alpha2-SNAPSHOT-bin.zip
I hope to check the actual artifacts and get back with any comments by
around Monday
cheers
asankha
--
Asankha C. Perera
AdroitLogic, http://adroitlogic.org
http://esbmagic.blogspot.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [HttpCore] HttpCore 4.1-beta1 release preview
Posted by "Asankha C. Perera" <as...@apache.org>.
Hi Oleg
Testing of the libraries were successful.
Would be good to update the Copyright years (in NOTICE file and
META-INF/NOTICEs) from "2006-2009" when you cut the final RC
thanks
asankha
--
Asankha C. Perera
AdroitLogic, http://adroitlogic.org
http://esbmagic.blogspot.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [HttpCore] HttpCore 4.1-beta1 release preview
Posted by James Leigh <ja...@leighnet.ca>.
On Wed, 2010-03-24 at 12:54 +0100, Oleg Kalnichevski wrote:
> On Tue, 2010-03-23 at 17:58 -0400, James Leigh wrote:
> > On Tue, 2010-03-23 at 22:06 +0100, Oleg Kalnichevski wrote:
> > > James Leigh wrote:
> > > > On Tue, 2010-03-23 at 20:41 +0100, Oleg Kalnichevski wrote:
> > > >> James Leigh wrote:
> > > >>> 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
> > > >>
> > > >
> > > > Unless Content-Length is zero, then the decoder is not complete until
> > > > after the first read method is called, which will return 0.
> > > >
> > >
> > > Would you consider it more appropriate for the codec to set its state to
> > > completed immediately if the content length is zero?
> > >
> > > Currently decoders of all types behave consistently. One need to do a
> > > read from a decoder, be it chunk, identity or content length delimited,
> > > in order to detect an empty stream. I would rather keep it this way.
> > >
> > > Oleg
> >
> > That makes sense, but I would expect the first read of a
> > known-to-be-empty stream to return -1 on the first read.
> >
>
> Fair enough. I committed a fix to the SVN trunk. Please have a look.
>
> http://svn.apache.org/viewvc?view=revision&revision=927019
>
> Oleg
>
Looks good thanks!
James
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [HttpCore] HttpCore 4.1-beta1 release preview
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Tue, 2010-03-23 at 17:58 -0400, James Leigh wrote:
> On Tue, 2010-03-23 at 22:06 +0100, Oleg Kalnichevski wrote:
> > James Leigh wrote:
> > > On Tue, 2010-03-23 at 20:41 +0100, Oleg Kalnichevski wrote:
> > >> James Leigh wrote:
> > >>> 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
> > >>
> > >
> > > Unless Content-Length is zero, then the decoder is not complete until
> > > after the first read method is called, which will return 0.
> > >
> >
> > Would you consider it more appropriate for the codec to set its state to
> > completed immediately if the content length is zero?
> >
> > Currently decoders of all types behave consistently. One need to do a
> > read from a decoder, be it chunk, identity or content length delimited,
> > in order to detect an empty stream. I would rather keep it this way.
> >
> > Oleg
>
> That makes sense, but I would expect the first read of a
> known-to-be-empty stream to return -1 on the first read.
>
Fair enough. I committed a fix to the SVN trunk. Please have a look.
http://svn.apache.org/viewvc?view=revision&revision=927019
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [HttpCore] HttpCore 4.1-beta1 release preview
Posted by James Leigh <ja...@leighnet.ca>.
On Tue, 2010-03-23 at 22:06 +0100, Oleg Kalnichevski wrote:
> James Leigh wrote:
> > On Tue, 2010-03-23 at 20:41 +0100, Oleg Kalnichevski wrote:
> >> James Leigh wrote:
> >>> 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
> >>
> >
> > Unless Content-Length is zero, then the decoder is not complete until
> > after the first read method is called, which will return 0.
> >
>
> Would you consider it more appropriate for the codec to set its state to
> completed immediately if the content length is zero?
>
> Currently decoders of all types behave consistently. One need to do a
> read from a decoder, be it chunk, identity or content length delimited,
> in order to detect an empty stream. I would rather keep it this way.
>
> Oleg
That makes sense, but I would expect the first read of a
known-to-be-empty stream to return -1 on the first read.
James
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [HttpCore] HttpCore 4.1-beta1 release preview
Posted by Oleg Kalnichevski <ol...@apache.org>.
James Leigh wrote:
> On Tue, 2010-03-23 at 20:41 +0100, Oleg Kalnichevski wrote:
>> 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
>>
>
> Unless Content-Length is zero, then the decoder is not complete until
> after the first read method is called, which will return 0.
>
Would you consider it more appropriate for the codec to set its state to
completed immediately if the content length is zero?
Currently decoders of all types behave consistently. One need to do a
read from a decoder, be it chunk, identity or content length delimited,
in order to detect an empty stream. I would rather keep it this way.
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [HttpCore] HttpCore 4.1-beta1 release preview
Posted by James Leigh <ja...@leighnet.ca>.
On Tue, 2010-03-23 at 20:41 +0100, Oleg Kalnichevski wrote:
> 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
>
Unless Content-Length is zero, then the decoder is not complete until
after the first read method is called, which will return 0.
James
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [HttpCore] HttpCore 4.1-beta1 release preview
Posted by Oleg Kalnichevski <ol...@apache.org>.
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
Re: [HttpCore] HttpCore 4.1-beta1 release preview
Posted by James Leigh <ja...@leighnet.ca>.
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
}
James
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [HttpCore] HttpCore 4.1-beta1 release preview
Posted by Oleg Kalnichevski <ol...@apache.org>.
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
Re: [HttpCore] HttpCore 4.1-beta1 release preview
Posted by James Leigh <ja...@leighnet.ca>.
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.
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?
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
Re: [HttpCore] HttpCore 4.1-beta1 release preview
Posted by Tushar Kapila <tu...@gmail.com>.
I agree * wait until we have enough important changes for a solid point zero
release and break binary compatibility at that point.
Or have a release now and then say no more binary compatibility for next
version but there might be patches for this release if any bug needs fixing.
That way you can start new API if required and still support this.
Re
> We cannot do that without breaking full binary compatibility with 4.0.x
> releases. As much as I consider binary compatibility for non trivial
> changes utterly pointless, I still think we should not break
> compatibility without a good reason. All these classes are not meant to
> be thread-safe, so mutability of instance variables is not really an
> issue.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [HttpCore] HttpCore 4.1-beta1 release preview
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Mon, 2010-03-22 at 22:56 +0000, sebb wrote:
> On 22/03/2010, Oleg Kalnichevski <ol...@apache.org> wrote:
> > On Sun, 2010-03-21 at 20:43 +0000, sebb wrote:
> > > On 21/03/2010, Oleg Kalnichevski <ol...@apache.org> 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
> > >
> > > Since this is the last chance to change the API, I have been having a
> > > look at the visibility and mutability of fields.
> > >
> >
> >
> > Hi Sebastian
> >
> > We cannot do that without breaking full binary compatibility with 4.0.x
> > releases. As much as I consider binary compatibility for non trivial
> > changes utterly pointless, I still think we should not break
> > compatibility without a good reason. All these classes are not meant to
> > be thread-safe, so mutability of instance variables is not really an
> > issue.
> >
> > We have a few options how to proceed:
> >
> > * deprecate those classes and create copies under a different name,
> > which we could restructure as we please. The trouble is that deprecation
> > of AbstractHttpEntity and AbstractHttpMessage will entail deprecation of
> > more than a dozen classes in HttpClient, which can be quite ugly.
> >
> > * release next release as 5.0-BETA1
> >
> > * wait until we have enough important changes for a solid point zero
> > release and break binary compatibility at that point.
> >
> > I suggest we take the last option for now.
>
> OK. I was considering 4.1 in isolation.
>
> I'll create at JIRA with patches to record the proposals.
>
> >
> > > AbstractHttpEntity
> > > has 3 protected fields, however there are public getters/setters for
> > > all of the fields, so it seems to me that the fields should be
> > > private. That would allow synch. to be added later if necessary.
> > >
> > > HttpEntityWrapper
> > > - wrappedEntity should be final.
> > >
> > > AbstractHttpMessage
> > > - headerGroup should be final
> > > - params should be private
> > >
> > > BasicHttpProcessor
> > > requestInterceptors and responseInterceptors should be final
> > > Should also be private, otherwise subclasses can subvert the non-null condition
> > >
> > > > Release notes:
> > > > http://people.apache.org/~olegk/httpcore-4.1-beta1-preview/RELEASE_NOTES.txt
> > >
> > > I did not quite understand the following sentence:
> > >
> > > "HttpCore based on
> > > classic (blocking) I/O model is expected to be 5% to 10% faster
> > > compared to previous releases. "
> > >
> > > Does this mean that previous releases were not based on the classic
> > > I/O model, and that changing to it has made the improvement?
> > >
> > > Or are these two independent facts:
> > > HttpCore is based on the classic (blocking) I/O model.
> > > This release is expected to be 5% to 10% faster than previous releases
> > >
> >
> >
> > Sorry I failed to articulate the idea properly. HttpCore comes in two
> > flavors: classic (blocking) I/O based and NIO based. Performance
> > improvements apply to the blocking components only. Please re-phrase the
> > statement as you see fit.
> >
>
> Your last change has fixed the ambiguity very well.
>
> >
> > > > Release packages:
> > > > http://people.apache.org/~olegk/httpcore-4.1-beta1-preview/
> > >
> > > DOAP probably does not belong in the source archive (or elsewhere).
> > >
> > > It's now 2010; Notice file needs updating.
> > >
> >
> >
> > I'll take care of that
> >
> > Oleg
> >
> > > Otherwise looks OK.
> >
> >
Folks,
I updated the release notes and the preview packages based on your
feedback. Feel free to double-check.
http://people.apache.org/~olegk/httpcore-4.1-beta1-preview/
I will start cutting official release packages tomorrow or the day after
tomorrow.
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [HttpCore] HttpCore 4.1-beta1 release preview
Posted by sebb <se...@gmail.com>.
On 22/03/2010, Oleg Kalnichevski <ol...@apache.org> wrote:
> On Sun, 2010-03-21 at 20:43 +0000, sebb wrote:
> > On 21/03/2010, Oleg Kalnichevski <ol...@apache.org> 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
> >
> > Since this is the last chance to change the API, I have been having a
> > look at the visibility and mutability of fields.
> >
>
>
> Hi Sebastian
>
> We cannot do that without breaking full binary compatibility with 4.0.x
> releases. As much as I consider binary compatibility for non trivial
> changes utterly pointless, I still think we should not break
> compatibility without a good reason. All these classes are not meant to
> be thread-safe, so mutability of instance variables is not really an
> issue.
>
> We have a few options how to proceed:
>
> * deprecate those classes and create copies under a different name,
> which we could restructure as we please. The trouble is that deprecation
> of AbstractHttpEntity and AbstractHttpMessage will entail deprecation of
> more than a dozen classes in HttpClient, which can be quite ugly.
>
> * release next release as 5.0-BETA1
>
> * wait until we have enough important changes for a solid point zero
> release and break binary compatibility at that point.
>
> I suggest we take the last option for now.
OK. I was considering 4.1 in isolation.
I'll create at JIRA with patches to record the proposals.
>
> > AbstractHttpEntity
> > has 3 protected fields, however there are public getters/setters for
> > all of the fields, so it seems to me that the fields should be
> > private. That would allow synch. to be added later if necessary.
> >
> > HttpEntityWrapper
> > - wrappedEntity should be final.
> >
> > AbstractHttpMessage
> > - headerGroup should be final
> > - params should be private
> >
> > BasicHttpProcessor
> > requestInterceptors and responseInterceptors should be final
> > Should also be private, otherwise subclasses can subvert the non-null condition
> >
> > > Release notes:
> > > http://people.apache.org/~olegk/httpcore-4.1-beta1-preview/RELEASE_NOTES.txt
> >
> > I did not quite understand the following sentence:
> >
> > "HttpCore based on
> > classic (blocking) I/O model is expected to be 5% to 10% faster
> > compared to previous releases. "
> >
> > Does this mean that previous releases were not based on the classic
> > I/O model, and that changing to it has made the improvement?
> >
> > Or are these two independent facts:
> > HttpCore is based on the classic (blocking) I/O model.
> > This release is expected to be 5% to 10% faster than previous releases
> >
>
>
> Sorry I failed to articulate the idea properly. HttpCore comes in two
> flavors: classic (blocking) I/O based and NIO based. Performance
> improvements apply to the blocking components only. Please re-phrase the
> statement as you see fit.
>
Your last change has fixed the ambiguity very well.
>
> > > Release packages:
> > > http://people.apache.org/~olegk/httpcore-4.1-beta1-preview/
> >
> > DOAP probably does not belong in the source archive (or elsewhere).
> >
> > It's now 2010; Notice file needs updating.
> >
>
>
> I'll take care of that
>
> Oleg
>
> > Otherwise looks OK.
>
>
>
>
> ---------------------------------------------------------------------
> 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
Re: [HttpCore] HttpCore 4.1-beta1 release preview
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Sun, 2010-03-21 at 20:43 +0000, sebb wrote:
> On 21/03/2010, Oleg Kalnichevski <ol...@apache.org> 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
>
> Since this is the last chance to change the API, I have been having a
> look at the visibility and mutability of fields.
>
Hi Sebastian
We cannot do that without breaking full binary compatibility with 4.0.x
releases. As much as I consider binary compatibility for non trivial
changes utterly pointless, I still think we should not break
compatibility without a good reason. All these classes are not meant to
be thread-safe, so mutability of instance variables is not really an
issue.
We have a few options how to proceed:
* deprecate those classes and create copies under a different name,
which we could restructure as we please. The trouble is that deprecation
of AbstractHttpEntity and AbstractHttpMessage will entail deprecation of
more than a dozen classes in HttpClient, which can be quite ugly.
* release next release as 5.0-BETA1
* wait until we have enough important changes for a solid point zero
release and break binary compatibility at that point.
I suggest we take the last option for now.
> AbstractHttpEntity
> has 3 protected fields, however there are public getters/setters for
> all of the fields, so it seems to me that the fields should be
> private. That would allow synch. to be added later if necessary.
>
> HttpEntityWrapper
> - wrappedEntity should be final.
>
> AbstractHttpMessage
> - headerGroup should be final
> - params should be private
>
> BasicHttpProcessor
> requestInterceptors and responseInterceptors should be final
> Should also be private, otherwise subclasses can subvert the non-null condition
>
> > Release notes:
> > http://people.apache.org/~olegk/httpcore-4.1-beta1-preview/RELEASE_NOTES.txt
>
> I did not quite understand the following sentence:
>
> "HttpCore based on
> classic (blocking) I/O model is expected to be 5% to 10% faster
> compared to previous releases. "
>
> Does this mean that previous releases were not based on the classic
> I/O model, and that changing to it has made the improvement?
>
> Or are these two independent facts:
> HttpCore is based on the classic (blocking) I/O model.
> This release is expected to be 5% to 10% faster than previous releases
>
Sorry I failed to articulate the idea properly. HttpCore comes in two
flavors: classic (blocking) I/O based and NIO based. Performance
improvements apply to the blocking components only. Please re-phrase the
statement as you see fit.
> > Release packages:
> > http://people.apache.org/~olegk/httpcore-4.1-beta1-preview/
>
> DOAP probably does not belong in the source archive (or elsewhere).
>
> It's now 2010; Notice file needs updating.
>
I'll take care of that
Oleg
> Otherwise looks OK.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [HttpCore] HttpCore 4.1-beta1 release preview
Posted by sebb <se...@gmail.com>.
On 21/03/2010, Oleg Kalnichevski <ol...@apache.org> 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
Since this is the last chance to change the API, I have been having a
look at the visibility and mutability of fields.
AbstractHttpEntity
has 3 protected fields, however there are public getters/setters for
all of the fields, so it seems to me that the fields should be
private. That would allow synch. to be added later if necessary.
HttpEntityWrapper
- wrappedEntity should be final.
AbstractHttpMessage
- headerGroup should be final
- params should be private
BasicHttpProcessor
requestInterceptors and responseInterceptors should be final
Should also be private, otherwise subclasses can subvert the non-null condition
> Release notes:
> http://people.apache.org/~olegk/httpcore-4.1-beta1-preview/RELEASE_NOTES.txt
I did not quite understand the following sentence:
"HttpCore based on
classic (blocking) I/O model is expected to be 5% to 10% faster
compared to previous releases. "
Does this mean that previous releases were not based on the classic
I/O model, and that changing to it has made the improvement?
Or are these two independent facts:
HttpCore is based on the classic (blocking) I/O model.
This release is expected to be 5% to 10% faster than previous releases
> Release packages:
> http://people.apache.org/~olegk/httpcore-4.1-beta1-preview/
DOAP probably does not belong in the source archive (or elsewhere).
It's now 2010; Notice file needs updating.
Otherwise looks OK.
> If I hear no complaints I will proceed with building the official release
> packages in a few days.
>
> Oleg
>
> ---------------------------------------------------------------------
> 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