tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rémy Maucherat <r...@apache.org>
Subject Re: [tomcat] branch master updated: Fix the HTTP/2 equivalent of swallowInput
Date Tue, 09 Apr 2019 11:14:19 GMT
On Tue, Apr 9, 2019 at 10:43 AM Mark Thomas <markt@apache.org> wrote:

> On 09/04/2019 08:50, Rémy Maucherat wrote:
>
> <snip/>
>
> > Thanks for the fix. I can indeed see that HttpParser.onHeadersComplete
> has:
> >         output.headersEnd(streamId); // <- dispatch is done here
> >
> >         if (headersEndStream) {
> >             output.receivedEndOfStream(streamId);
> >             headersEndStream = false;
> >         }
> >
> > I guess it's possible to rely on syncing but normally thread dispatching
> > should not occur until after the state is properly set, it's simply
> safer.
>
> I did think about swapping the order of those statements. When I tested
> it I saw one test failure with trailer headers (I didn't investigate
> further) so I went for a different solution.
>

There is a failure indeed, but it's a logging issue only it seems (the
callback log the events in order, so it doesn't match the comparison string
anymore).


>
> It may be the swapping the order is safe but that would need more
> investigation. Also, triggering EOS before end of headers just seems
> wrong to me.
>

I agree it seems wrong. There's another occurrence of the behavior though,
in Http2Parser.readDataFrame, where it does:
                if (endOfStream) {
                    output.receivedEndOfStream(streamId);
                }
                output.endRequestBodyFrame(streamId);

I will try to investigate, but only after the next build, it's clearly not
worth breaking something.

Rémy

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message