RFR: 8309118: HttpClient: Add more tests for 100 ExpectContinue with HTTP/2 [v3]

Daniel Fuchs dfuchs at openjdk.org
Wed Oct 18 15:02:06 UTC 2023


On Wed, 18 Oct 2023 14:56:55 GMT, Conor Cleary <ccleary at openjdk.org> wrote:

>> src/java.net.http/share/classes/jdk/internal/net/http/Stream.java line 193:
>> 
>>> 191:                 if (frame instanceof ResetFrame rf) {
>>> 192:                     inputQ.remove();
>>> 193:                     if (endStreamReceived()) {
>> 
>> Suggestion:
>> 
>>                     if (endStreamReceived() && rf.getErrorCode() == ResetFrame.NO_ERROR) {
>
> So if we have received an end stream but receive an error code, do we want to complete exceptionally? 
> 
> The suggestion seems correct, just checking 👍

Yes - I believe so - the peer indicates an error - so we should complete exceptionally, if that's still possible.

>> src/java.net.http/share/classes/jdk/internal/net/http/Stream.java line 195:
>> 
>>> 193:                     if (endStreamReceived()) {
>>> 194:                         // If END_STREAM is already received, we should not receive any new RST_STREAM frames and
>>> 195:                         // close the connection gracefully by processing all remaining frames in the inputQ.
>> 
>> Suggestion:
>> 
>>                         // If END_STREAM is already received, we should we should simply
>>                         // complete the requestBodyCF successfuly and stop sending any
>>                         // request data.
>
> How about just
> 
> // If END_STREAM is already received, complete the requestBodyCF successfully
> // and stop sending any request data.

works for me

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15664#discussion_r1364035697
PR Review Comment: https://git.openjdk.org/jdk/pull/15664#discussion_r1364030258


More information about the net-dev mailing list