You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Rémy Maucherat <re...@apache.org> on 2025/01/30 09:36:51 UTC
Re: Buildbot failure in on tomcat-11.0.x
On Thu, Jan 30, 2025 at 12:26 AM <bu...@apache.org> wrote:
>
> Build status: BUILD FAILED: failed compile (failure)
> Worker used: bb_worker2_ubuntu
> URL: https://ci2.apache.org/#builders/112/builds/1490
> Blamelist: remm <re...@apache.org>
> Build Text: failed compile (failure)
> Status Detected: new failure
> Build Source Stamp: [branch 11.0.x] 86862a7544bded47734b565e19de5f1b7e8f5ea1
It seems I found an issue in ChunkedInputFilter in the end.
It was working fine for me locally before I committed, but only
because I had a different code in available() (going for a less
hackish way to skip the CRLF - and also the chunk header).
Simply removing the code that was added to available() for CRLF also
fixes the test.
Investigating ... [but this is very mysterious for now]
Rémy
>
> Steps:
>
> worker_preparation: 0
>
> git: 0
>
> shell: 0
>
> shell_1: 0
>
> shell_2: 0
>
> shell_3: 0
>
> shell_4: 0
>
> shell_5: 0
>
> shell_6: 0
>
> compile: 1
>
> shell_7: 0
>
> shell_8: 0
>
> shell_9: 0
>
> shell_10: 0
>
> Rsync docs to nightlies.apache.org: 0
>
> shell_11: 0
>
> Rsync RAT to nightlies.apache.org: 0
>
> compile_1: 2
>
> shell_12: 0
>
> Rsync Logs to nightlies.apache.org: 0
>
>
> -- ASF Buildbot
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Buildbot failure in on tomcat-11.0.x
Posted by Mark Thomas <ma...@apache.org>.
On 30/01/2025 16:04, Rémy Maucherat wrote:
> On Thu, Jan 30, 2025 at 4:35 PM Mark Thomas <ma...@apache.org> wrote:
>>
>> On 30/01/2025 11:20, Rémy Maucherat wrote:
>>> On Thu, Jan 30, 2025 at 12:10 PM Mark Thomas <ma...@apache.org> wrote:
>>>>
>>>> On 30/01/2025 10:32, Rémy Maucherat wrote:
>>>>> Yes, there's an off by one issue, still don't understand what's
>>>>> causing it (anything that gets into the available = 0 situations in
>>>>> available() will break). My local code replaces the CRLF trick with
>>>>> proper byte skipping and does not have the issue. I'll commit it to
>>>>> see if the testsuite on CI is ok with it.
>>>>
>>>> I've made some progress.
>>>>
>>>> The readChunk is being changed outside of the ChunkedInputFilter during
>>>> the pause between writes from the client. I'm still tracking down the
>>>> root cause.
>>>
>>> Ok. I committed my code a little bit earlier for testing.
>>
>> The root cause was the the ChunkedInputFilter returning 0 bytes
>> available when there were unconsumed bytes in readChunk. That caused
>> Http11InputBuffer to read more data which over-wrote the unread bytes in
>> ChunkedInputFilfer.readChunk
>>
>> The ChunkedInputFilfer and Http11InputBuffer were then out of sync
>> leading to the input corruption.
>>
>> Your fix, which ensures all the bytes in ChunkedInputFilfer.readChunk
>> are consumed, should fix this and any related issues.
>
> I had to pause due to a storm with lots of lightning.
Yikes. I hope everyone is OK.
> I intuitively understand that returning 0 for available does trigger a
> read in the end. I also saw during debug that the buffer position got
> updated backwards, so bytes would change then. As a result, there was
> no way to avoid processing the bytes before reporting 0, and my patch
> is more correct. I think it is possible to safely skip (and process) a
> chunk header in available(), but processing trailers is not very
> useful as EOF will follow.
> > So the new code consumes all irrelevant bytes unless there's an error
> or guaranteed EOF, in which case it says there's something available.
> The next read will then throw an exception or return -1. Unfortunately
> it was duplicating code since the regular code path throws and so on.
> I was not expecting to commit it.
Working with duplicated code is better than not working.
I did notice the duplication and wondered if there was a way to reduce
it but I didn't spend any time on that. I might look at that while I am
running the unit tests over the next couple of days.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Buildbot failure in on tomcat-11.0.x
Posted by Rémy Maucherat <re...@apache.org>.
On Thu, Jan 30, 2025 at 4:35 PM Mark Thomas <ma...@apache.org> wrote:
>
> On 30/01/2025 11:20, Rémy Maucherat wrote:
> > On Thu, Jan 30, 2025 at 12:10 PM Mark Thomas <ma...@apache.org> wrote:
> >>
> >> On 30/01/2025 10:32, Rémy Maucherat wrote:
> >>> Yes, there's an off by one issue, still don't understand what's
> >>> causing it (anything that gets into the available = 0 situations in
> >>> available() will break). My local code replaces the CRLF trick with
> >>> proper byte skipping and does not have the issue. I'll commit it to
> >>> see if the testsuite on CI is ok with it.
> >>
> >> I've made some progress.
> >>
> >> The readChunk is being changed outside of the ChunkedInputFilter during
> >> the pause between writes from the client. I'm still tracking down the
> >> root cause.
> >
> > Ok. I committed my code a little bit earlier for testing.
>
> The root cause was the the ChunkedInputFilter returning 0 bytes
> available when there were unconsumed bytes in readChunk. That caused
> Http11InputBuffer to read more data which over-wrote the unread bytes in
> ChunkedInputFilfer.readChunk
>
> The ChunkedInputFilfer and Http11InputBuffer were then out of sync
> leading to the input corruption.
>
> Your fix, which ensures all the bytes in ChunkedInputFilfer.readChunk
> are consumed, should fix this and any related issues.
I had to pause due to a storm with lots of lightning.
I intuitively understand that returning 0 for available does trigger a
read in the end. I also saw during debug that the buffer position got
updated backwards, so bytes would change then. As a result, there was
no way to avoid processing the bytes before reporting 0, and my patch
is more correct. I think it is possible to safely skip (and process) a
chunk header in available(), but processing trailers is not very
useful as EOF will follow.
So the new code consumes all irrelevant bytes unless there's an error
or guaranteed EOF, in which case it says there's something available.
The next read will then throw an exception or return -1. Unfortunately
it was duplicating code since the regular code path throws and so on.
I was not expecting to commit it.
Rémy
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Buildbot failure in on tomcat-11.0.x
Posted by Mark Thomas <ma...@apache.org>.
On 30/01/2025 11:20, Rémy Maucherat wrote:
> On Thu, Jan 30, 2025 at 12:10 PM Mark Thomas <ma...@apache.org> wrote:
>>
>> On 30/01/2025 10:32, Rémy Maucherat wrote:
>>> Yes, there's an off by one issue, still don't understand what's
>>> causing it (anything that gets into the available = 0 situations in
>>> available() will break). My local code replaces the CRLF trick with
>>> proper byte skipping and does not have the issue. I'll commit it to
>>> see if the testsuite on CI is ok with it.
>>
>> I've made some progress.
>>
>> The readChunk is being changed outside of the ChunkedInputFilter during
>> the pause between writes from the client. I'm still tracking down the
>> root cause.
>
> Ok. I committed my code a little bit earlier for testing.
The root cause was the the ChunkedInputFilter returning 0 bytes
available when there were unconsumed bytes in readChunk. That caused
Http11InputBuffer to read more data which over-wrote the unread bytes in
ChunkedInputFilfer.readChunk
The ChunkedInputFilfer and Http11InputBuffer were then out of sync
leading to the input corruption.
Your fix, which ensures all the bytes in ChunkedInputFilfer.readChunk
are consumed, should fix this and any related issues.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Buildbot failure in on tomcat-11.0.x
Posted by Rémy Maucherat <re...@apache.org>.
On Thu, Jan 30, 2025 at 12:10 PM Mark Thomas <ma...@apache.org> wrote:
>
> On 30/01/2025 10:32, Rémy Maucherat wrote:
> > Yes, there's an off by one issue, still don't understand what's
> > causing it (anything that gets into the available = 0 situations in
> > available() will break). My local code replaces the CRLF trick with
> > proper byte skipping and does not have the issue. I'll commit it to
> > see if the testsuite on CI is ok with it.
>
> I've made some progress.
>
> The readChunk is being changed outside of the ChunkedInputFilter during
> the pause between writes from the client. I'm still tracking down the
> root cause.
Ok. I committed my code a little bit earlier for testing.
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Buildbot failure in on tomcat-11.0.x
Posted by Mark Thomas <ma...@apache.org>.
On 30/01/2025 10:32, Rémy Maucherat wrote:
> Yes, there's an off by one issue, still don't understand what's
> causing it (anything that gets into the available = 0 situations in
> available() will break). My local code replaces the CRLF trick with
> proper byte skipping and does not have the issue. I'll commit it to
> see if the testsuite on CI is ok with it.
I've made some progress.
The readChunk is being changed outside of the ChunkedInputFilter during
the pause between writes from the client. I'm still tracking down the
root cause.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Buildbot failure in on tomcat-11.0.x
Posted by Rémy Maucherat <re...@apache.org>.
On Thu, Jan 30, 2025 at 11:21 AM Mark Thomas <ma...@apache.org> wrote:
>
> On 30/01/2025 09:36, Rémy Maucherat wrote:
> > On Thu, Jan 30, 2025 at 12:26 AM <bu...@apache.org> wrote:
> >>
> >> Build status: BUILD FAILED: failed compile (failure)
> >> Worker used: bb_worker2_ubuntu
> >> URL: https://ci2.apache.org/#builders/112/builds/1490
> >> Blamelist: remm <re...@apache.org>
> >> Build Text: failed compile (failure)
> >> Status Detected: new failure
> >> Build Source Stamp: [branch 11.0.x] 86862a7544bded47734b565e19de5f1b7e8f5ea1
> >
> > It seems I found an issue in ChunkedInputFilter in the end.
> >
> > It was working fine for me locally before I committed, but only
> > because I had a different code in available() (going for a less
> > hackish way to skip the CRLF - and also the chunk header).
> > Simply removing the code that was added to available() for CRLF also
> > fixes the test.
> >
> > Investigating ... [but this is very mysterious for now]
>
> I'm looking too. Looks like an off by one issue somewhere but I can't
> see where yet. Need to add more local debug logging to ChunkedInputFilter.
Yes, there's an off by one issue, still don't understand what's
causing it (anything that gets into the available = 0 situations in
available() will break). My local code replaces the CRLF trick with
proper byte skipping and does not have the issue. I'll commit it to
see if the testsuite on CI is ok with it.
Note: TestChunkedInputFilter is skipped in the GH actions due to
taking too much time to complete, which is why I didn't immediately
notice it.
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Buildbot failure in on tomcat-11.0.x
Posted by Mark Thomas <ma...@apache.org>.
On 30/01/2025 09:36, Rémy Maucherat wrote:
> On Thu, Jan 30, 2025 at 12:26 AM <bu...@apache.org> wrote:
>>
>> Build status: BUILD FAILED: failed compile (failure)
>> Worker used: bb_worker2_ubuntu
>> URL: https://ci2.apache.org/#builders/112/builds/1490
>> Blamelist: remm <re...@apache.org>
>> Build Text: failed compile (failure)
>> Status Detected: new failure
>> Build Source Stamp: [branch 11.0.x] 86862a7544bded47734b565e19de5f1b7e8f5ea1
>
> It seems I found an issue in ChunkedInputFilter in the end.
>
> It was working fine for me locally before I committed, but only
> because I had a different code in available() (going for a less
> hackish way to skip the CRLF - and also the chunk header).
> Simply removing the code that was added to available() for CRLF also
> fixes the test.
>
> Investigating ... [but this is very mysterious for now]
I'm looking too. Looks like an off by one issue somewhere but I can't
see where yet. Need to add more local debug logging to ChunkedInputFilter.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org