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

Aleksei Efimov aefimov at openjdk.org
Thu Oct 6 17:08:40 UTC 2022


On Thu, 6 Oct 2022 11:35:32 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

The fix looks reasonable to me, with one comment regarding the test cleanup, and a couple of minor formatting issues.

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

> 754:         if(uri.getHost() != null && uri.getPort() != -1) {
> 755:             hdrs.setHeader(":authority", uri.getHost() + ":" + uri.getPort());
> 756:         } else if (uri.getHost()!=null) {

Formatting: missing spaces: `uri.getHost() != null`

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

> 1: /*
> 2:  * Copyright (c) 2022, 2022, Oracle and/or its affiliates. All rights reserved.

Since it is a new test, only the file creation date needs to be specified in the copyright header: `Copyright (c) 2022, Oracle`

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

> 46: public class UserInfoTest {
> 47: 
> 48:     static class Http2TestHandler implements Http2Handler{

Formatting: missing a space before the open brace: `Http2Handler {`

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

> 56:             is.readAllBytes();
> 57:             is.close();
> 58:             if(e.getRequestURI().getAuthority().contains("user@")){

Formatting: missing spaces before open brace and after if:
 `if (e.getRequestURI().getAuthority().contains("user@")) {`

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

> 65: 
> 66:     private static Http2TestServer createServer(SSLContext sslContext) throws Exception {
> 67:         Http2TestServer http2TestServer = new Http2TestServer("localhost",true, sslContext);

Formatting: missing a space between 2nd and 3rd ctor parameter:
`new Http2TestServer("localhost", true, sslContext);`

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

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

Formatting: missing spaces: `response.statusCode() == 500`

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

> 95:             throw new RuntimeException("Test Failed : " + response.uri().getAuthority());
> 96:         }
> 97:         server.stop();

Since the test is running in agentvm mode then it is better to stop the test server once the test method completed execution: One possibility is to wrap http request code and the status code check into `try {} finally{}` and stop the server even if the test failed with `RuntimeException`.

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

> 97:         server.stop();
> 98:     }
> 99: }

Formatting: No new line at the end of file

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

Changes requested by aefimov (Committer).

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


More information about the net-dev mailing list