RFR: 8054022: HttpURLConnection timeouts with Expect: 100-Continue and no chunking
Daniel Jeliński
djelinski at openjdk.org
Wed Apr 5 09:34:19 UTC 2023
On Tue, 4 Apr 2023 17:02:08 GMT, Darragh Clarke <duke at openjdk.org> wrote:
> Currently it is possible for `HttpURLConnection` with the `Expect: 100-Continue` header to timeout awaiting for a server response. According to [RFC-7231](https://www.rfc-editor.org/rfc/rfc7231#section-5.1.1) a client `SHOULD NOT wait for an indefinite period before sending the message body`.
>
> This PR changes the existing `expect100Continue` method to wait for a maximum of 5 seconds for a server response, this will be shorter if a timeout is set. If no response is received, the message is sent regardless.
>
> Tests have been added to account for different scenarios that currently timeout, and the changes have been tested against tiers 1,2 and 3.
Changes requested by djelinski (Reviewer).
src/java.base/share/classes/sun/net/www/protocol/http/HttpURLConnection.java line 1370:
> 1368: }
> 1369:
> 1370: http.setIgnoreContinue(true);
Do you still need the `setIgnoreContinue(true)` in line 1339?
src/java.base/share/classes/sun/net/www/protocol/http/HttpURLConnection.java line 1453:
> 1451:
> 1452: if (expectContinue) {
> 1453: http.setIgnoreContinue(false);
Do you still need the `setIgnoreContinue(false)` a few lines above?
test/jdk/java/net/HttpURLConnection/HttpURLConnectionExpectContinueTest.java line 1:
> 1: /*
I noticed that you have tests for chunked streaming mode and for buffered mode; can you add tests for fixed length streaming mode as well? I.e. use `setFixedLengthStreamingMode`.
test/jdk/java/net/HttpURLConnection/HttpURLConnectionExpectContinueTest.java line 114:
> 112: int contentLength = Integer.parseInt(substr.trim());
> 113:
> 114: if (control.respondWith100Continue) {
this code is duplicated in the `if` and `else` branches; move it outside.
-------------
PR Review: https://git.openjdk.org/jdk/pull/13330#pullrequestreview-1372246778
PR Review Comment: https://git.openjdk.org/jdk/pull/13330#discussion_r1158113297
PR Review Comment: https://git.openjdk.org/jdk/pull/13330#discussion_r1158084804
PR Review Comment: https://git.openjdk.org/jdk/pull/13330#discussion_r1158245845
PR Review Comment: https://git.openjdk.org/jdk/pull/13330#discussion_r1158237770
More information about the net-dev
mailing list