RFR: 8292876: Do not include the deprecated userinfo component of the URI in HTTP/2 headers [v2]

Daniel Fuchs dfuchs at openjdk.org
Fri Oct 7 16:24:30 UTC 2022


On Fri, 7 Oct 2022 11:18:15 GMT, Darragh Clarke <duke at openjdk.org> wrote:

>> Changed the way the `:authority` pseudo header is set to only include host and, if available, port.
>> I added a test to cover this change that consists of a HttpClient that makes a request which contains userInfo, the test passes if the request is carried out with the userInfo not being added to the `:authority` header.
>> 
>> 
>> ### Tests
>> I ran Tier 1 - Tier 3 tests, as well as paying special attention to the http client tests to make sure they consistently passed
>
> Darragh Clarke has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fixed formatting, added finally to cleanup server if test fails

Changes requested by dfuchs (Reviewer).

src/java.net.http/share/classes/jdk/internal/net/http/Stream.java line 754:

> 752:         URI uri = request.uri();
> 753:         hdrs.setHeader(":scheme", uri.getScheme());
> 754:         if (uri.getHost() != null && uri.getPort() != -1) {

I would suggest creating local variables for `host` and `port` in order to avoid calling getHost/getPort multiple times. It will also make possible to `assert host != null` since that's what we expect (it should not reach here if getHost() returns null)

test/jdk/java/net/httpclient/http2/UserInfoTest.java line 82:

> 80: 
> 81:         HttpClient client = HttpClient
> 82:                 .newBuilder()

It's always better to make sure that no proxy will be selected when building an `HttpClient` - unless the test has a proxy that should be used.

test/jdk/java/net/httpclient/http2/UserInfoTest.java line 95:

> 93:             HttpResponse<String> response = client.send(request, HttpResponse.BodyHandlers.ofString());
> 94: 
> 95:             if (response.statusCode() == 500) {

I believe the correct test here is `if (response.statusCode() != 200)` - if we receive anything other than 200 it is unexpected and the test should fail.

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

PR: https://git.openjdk.org/jdk/pull/10592


More information about the net-dev mailing list