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