On Wed, 4 May 2022 11:17:32 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> **Issue** >> A recent fix for >> [JDK-8263031](https://bugs.openjdk.java.net/browse/JDK-8263031), which >> addressed the httpclient being unable to properly process Http/2 PushPromise >> frames followed by continuations caused intermittent failures of the test. >> This was cause by two seperate problems. >> >> - Firstly, on the client side, `Http2Connection.processFrame` did not a >> check for whether or not a Continuation was to be expected early on in the >> method resulting in frames other than the expected Continuation Frame being >> incorrectly processed. In Http/2, when a Continuation Frame is expected, no >> other type of frame should be processed without a connection error of type >> PROTOCOL_ERROR being thrown. >> - Secondly, `Http2TestConnection.handlePush` had no guards around response >> headers and data frames being sent before a continuation is sent which >> resulted in the intermittent nature of this timeout. >> >> **Proposed Solution** >> The test class `OutgoingPushPromise` was modified to contain a list of >> Continuation Frames as required rather than the Continuations being >> scheduled to send in the test itself. This prevents the situation of >> unexpected frames being sent before a Continuation Frame was meant to >> arrive. >> >> In addition to the above, Http2Connection was modified to ensure that a >> connection error of type PROTOCOL_ERROR is thrown if a frame other than a >> Continuation arrives at the client unexpectedly. > > test/jdk/java/net/httpclient/http2/server/OutgoingPushPromise.java line 56: > >> 54: continuations = new LinkedList<>(); >> 55: continuations.add(cf); >> 56: } > > I would suggest adding a constructor that takes a list of > continuations/frames instead. This way you could have an immutable list (use > List.of/List.copyOf) Ah, good suggestion! ------------- PR: https://git.openjdk.java.net/jdk/pull/8518
