On Tue, 11 Oct 2022 11:31:37 GMT, Darragh Clarke <[email protected]> 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 test

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

> 57:             for (int ch; (ch = is.read()) != -1; ) {
> 58:                 sb.append((char) ch);
> 59:             }

Is this a left over code perhaps? You could perhaps just replace this 
completely with:


try(InputStream is = e.getRequestBody()) {
    is.readAllBytes();
}

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

> 76: 
> 77:     @Test
> 78:     public void main() throws Exception {

Nit - perhaps just rename the method to something other than `main`? Because 
anyway, this can't be used as a `main` method without changing its signature to 
accept a `String[]`. Perhaps call it `testAuthorityHeader`? instead?

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

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

Reply via email to