On Mon, 30 Jun 2025 11:18:52 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:

>> Jaikiran Pai has updated the pull request incrementally with four additional 
>> commits since the last revision:
>> 
>>  - SNI server names can now be derived from the Origin instance
>>  - strip the square brackets from URI's host when constructing an Origin
>>  - support only lower case http and https literals for scheme in Origin
>>  - add new line
>
> src/java.net.http/share/classes/jdk/internal/net/http/AsyncSSLConnection.java 
> line 50:
> 
>> 48:                        String[] alpn,
>> 49:                        String label) {
>> 50:         super(originServer, addr, client, Utils.getServerName(addr), 
>> addr.getPort(), alpn, label);
> 
> Do we still need Utils.getServerName(addr), addr.getPort(), now that we have 
> originServer? Or in other words - should we base these calls on the `addr` 
> parameter or on the `originServer` parameters?

I had a look at it and you are right - with the introduction of this `Origin` 
construct, it should now be possible (and in fact prefered) to use it for 
constructing the SNI names. I updated this code to use the `Origin` instance. 

While doing that, the change caused a test failure which exposed one detail 
that I hadn't considered. `java.net.URI.getHost()` is specified to return a 
IPv6 address enclosed in square brackets. The `Origin` class hadn't considered 
that previously. It's now been updated to strip the square brackets when 
constructing the host value. This will allow the `Origin.host()` to be used in 
places where it was/is being used as a host that might be returned from a 
`InetAddress`. Local testing with this change now passes all the tests. I'll 
now trigger tier testing in our CI.

> src/java.net.http/share/classes/jdk/internal/net/http/AsyncSSLTunnelConnection.java
>  line 54:
> 
>> 52:                              String label)
>> 53:     {
>> 54:         super(originServer, addr, client, Utils.getServerName(addr), 
>> addr.getPort(), alpn, label);
> 
> Same question

Updated to use the `Origin` instance.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26041#discussion_r2175349780
PR Review Comment: https://git.openjdk.org/jdk/pull/26041#discussion_r2175351647

Reply via email to