On Wed, 16 Oct 2024 10:48:46 GMT, Michael McMahon <micha...@openjdk.org> wrote:
>> Hi, >> >> I closed https://github.com/openjdk/jdk/pull/21249 and am continuing the >> review on this PR. >> >> This fix relaxes the constraints on user set authentication headers. >> Currently, any user set authentication headers are filtered out, if the >> HttpClient has an Authenticator set. The reason being that the authenticator >> is expected to manage authentication. With this fix, it will be possible to >> use pre-emptive authentication through user set headers, even if an >> authenticator is set. The expected use case is where the authenticator would >> manage either proxy or server authentication and the user set headers would >> manage server authentication if the authenticator is managing proxy (or vice >> versa). >> >> A CSR will be filed to document this change. >> >> Thanks, >> Michael > > Michael McMahon has updated the pull request incrementally with one > additional commit since the last revision: > > updated apidoc test/jdk/java/net/httpclient/UserAuthWithAuthenticator.java line 137: > 135: .executor(ex); > 136: > 137: //if (!useHeader) { do you plan to keep this commented-out code? test/jdk/java/net/httpclient/UserAuthWithAuthenticator.java line 148: > 146: .build(); > 147: > 148: HttpResponse<String> resp = client.send(req, > HttpResponse.BodyHandlers.ofString()); Can you assert that the response code is as expected? test/jdk/java/net/httpclient/UserAuthWithAuthenticator.java line 150: > 148: HttpResponse<String> resp = client.send(req, > HttpResponse.BodyHandlers.ofString()); > 149: if (useHeader) { > 150: assertTrue(h.authValue() == null, "Expected user set > header to be set"); authValue is null if the handler was never called; is that what we want to check here? test/jdk/java/net/httpclient/UserAuthWithAuthenticator.java line 154: > 152: System.out.println("h2Test: using user set header OK"); > 153: } else { > 154: assertTrue(!h.authValue().equals(encoded), "Expected > user set header to not be set"); could you check that the authValue is equal to the expected value? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21408#discussion_r1803139315 PR Review Comment: https://git.openjdk.org/jdk/pull/21408#discussion_r1803146782 PR Review Comment: https://git.openjdk.org/jdk/pull/21408#discussion_r1803141600 PR Review Comment: https://git.openjdk.org/jdk/pull/21408#discussion_r1803144480