On Thu, 12 May 2022 11:54:55 GMT, Conor Cleary <ccle...@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.
>
> Conor Cleary has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8284585: Added test case to verify fix

Please make sure the test is stable with these changes, and if it is then LGTM.

-------------

Marked as reviewed by dfuchs (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8518

Reply via email to