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