RFR: 8376031: HttpsURLConnection.getServerCertificates() throws "java.lang.IllegalStateException: connection not yet open" for the HEAD method

[Daniel Fuchs]

On Sun, 1 Feb 2026 11:56:11 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> The issue here is that `HttpURLConnection` is automatically disconnected (`HttpClient` is set to `null`, `connected` is set to `false`) when a response with no response body bytes is received. This happens before a fake empty body input stream is returned to the user. That behaviour also occurs with any method for which `content-length: 0` is returned (GET, POST, custom, anything), and with any status code (204, 304) for which there is no body.
>> 
>> In this case, the proposed fix is to store the `SSLSession` in the `AbstractDelegateHttpsURLConnection` subclass until such a time where `disconnect()` is explicitely closed. Information pertaining to SSL, such as server certificates, can be extracted from the saved `SSLSession`.
>
> test/jdk/sun/net/www/protocol/https/HttpsURLConnection/GetServerCertificates.java line 140:
> 
>> 138:         if (!"GET".equals(method)) {
>> 139:             uc.setRequestMethod(method);
>> 140:         }
> 
> Is the use of these conditionals on `GET` intentional? From what I can see, we could just call `uc.setRequestMethod(method);` without the `if` blocks. It's OK to keep it in this manner if you prefer doing so.

For GET the regular path would be to not set the method - I preferred to keep the regular path.

> test/jdk/sun/net/www/protocol/https/HttpsURLConnection/GetServerCertificates.java line 151:
> 
>> 149:                 .formatted(code, resp));
>> 150: 
>> 151:         uc.getServerCertificates();
> 
> Here and a few other places in this test, should we assert that this returns non-null certificates?

I guess we could - but if the bug is not fixed an exception would be thrown there. Checking the returned certificates didn't look to gain us much.

> test/jdk/sun/net/www/protocol/https/HttpsURLConnection/GetServerCertificates.java line 295:
> 
>> 293:         try {test(args);} catch (Throwable t) {unexpected(t);}
>> 294:         System.out.printf("%nPassed = %d, failed = %d%n%n", passed, failed);
>> 295:         if (failed > 0) throw new AssertionError("Some tests failed");}
> 
> I realize this was the old style of tests we have in some parts of the JDK, but maybe for this new test we can simplify this to just something like:
> 
> public static void main(String[] args) throws Exception {
>      tests(args);
> }
> 
> and then let any `check()` calls throw and propagate an exception instead of counting the number of pass/fail?

This is copy paste from a nearby test - I first attempted to remove it but then discovered that I would either have to hard code the name of the class to make a new instance (new GetServerCertificates()) or make all instance variables and methods static. In the end I decided to keep it.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29489#discussion_r2759569145
PR Review Comment: https://git.openjdk.org/jdk/pull/29489#discussion_r2759573880
PR Review Comment: https://git.openjdk.org/jdk/pull/29489#discussion_r2759559796