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

Daniel Fuchs dfuchs at openjdk.org
Tue Oct 11 13:39:18 UTC 2022


On Tue, 11 Oct 2022 12:11:57 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Darragh Clarke has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fixed test
>
> test/jdk/java/net/httpclient/http2/UserInfoTest.java line 62:
> 
>> 60:             is.readAllBytes();
>> 61:             is.close();
>> 62:             if (e.getRequestURI().getAuthority().contains("user@")) {
> 
> I think we should be checking the value of the `:authority` header here. I haven't yet checked the test server code to see how we construct the URI returned by `Http2TestExchange.getRequestURI`, but even if we used the `:authority` header to create that URI, I still think we should be checking the request headers to verify that the right request header value is received.

Yes - good catch! I missed that. The Http2TestServerConnection does that:

            String us = scheme + "://" + authority + path;
            URI uri = new URI(us);

but checking the `:authority` pseudo header would be cleaner.

> test/jdk/java/net/httpclient/http2/UserInfoTest.java line 96:
> 
>> 94:                 .loopback()
>> 95:                 .port(port)
>> 96:                 .build();
> 
> Should we explicitly set the version to `HTTP_2` here to make it clear that we are interested in HTTP2 `:authority` header?

HTTP_2 is the default and our server is a TLS server supporting only HTTP/2 so it doesn't seem necessary.
For the record we're using TLS here to avoid the upgrade, because for the upgrade request the ":authority" pseudo header is reconstituted on the server side by the test server from the HTTP/1.1 host header - and that's not the code we want to test.

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

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


More information about the net-dev mailing list