On Wed, 9 Apr 2025 11:30:58 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Overhauls `EmptyAuthenticate` to
>>
>> - Test all supported HTTP versions (i.e., HTTP/1.1 and HTTP/2)
>> - Test both clear-text and SSL
>> - Use `HttpServerAdapters.HttpTestServer::create` to avoid host-related
>> problems
>
> test/jdk/java/net/httpclient/EmptyAuthenticate.java line 83:
>
>> 81:
>> responseHeaders.firstValue(WWW_AUTH_HEADER_NAME).orElse(null),
>> 82: () -> "was expecting empty `%s` header in:
>> %s".formatted(
>> 83: WWW_AUTH_HEADER_NAME,
>> responseHeaders.map()));
>
> Hello Volkan, this appears to be the opposite of what the original test case
> was expecting. Before this change, we had:
>
> boolean ok = !response.headers().firstValue("WWW-Authenticate").isEmpty();
> if (!ok) {
> throw new RuntimeException("WWW-Authenticate missing");
> }
>
> So the test was expecting the response header value to be non-empty unlike
> this updated version. Did I misread the code?
I'm guessing that this test passes for you locally. If so, then we might need
some additional investigation here as to why it passes (given the exact
opposite assertion of what the test was previously asserting).
I see that the original test was using lower case response header in the server
handler and on the client side the test was reading a upper case response
header. In this updated version, we use the upper case header consistently in
both the handler and the client. You might want to check the original issue
https://bugs.openjdk.org/browse/JDK-8263899 to see if the case of the header
was playing a role in this test.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24542#discussion_r2035189071
