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