On Tue, 17 Jun 2025 06:55:38 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

> Can I please get a review for this change which addresses a regression that 
> was introduced in `HttpURLConnection` in Java 24 when we cleaned up the code 
> by removing the references to SecurityManager APIs.
> 
> When a HTTP request is issued through `java.net.HttpURLConnection`, then the 
> request URL is used to determine the `Host` header to set in the request. By 
> default, the application cannot set a `Host` header to a different value. 
> However the JDK allows a system property to be enabled to allow applications 
> to explicitly set a `Host` request header when issuing the request.
> 
> Due to an oversight in the change that was done in 
> https://bugs.openjdk.org/browse/JDK-8344190, the `Host` header that is set by 
> the application, may not get used for that request causing this regression. 
> Turns out we don't have tests in this area to catch this issue.
> 
> The commit in this PR fixes the regression and also introduces a new jtreg 
> test which reproduces the issue and verifies the fix.
> 
> I've also checked the original change which introduced this regression 
> https://github.com/openjdk/jdk/pull/22232 to see if there's anything else 
> that needs attention. I haven't stopped anything else.

Marked as reviewed by vyazici (Committer).

test/jdk/java/net/HttpURLConnection/HostHeaderTest.java line 68:

> 66:         final InetSocketAddress addr = new 
> InetSocketAddress(InetAddress.getLoopbackAddress(), 0);
> 67:         server = HttpServer.create(addr, 0);
> 68:         server.createContext("/", new Handler());

I think it might be a good idea to salt the handler path a bit (e.g., with the 
class name) to avoid unexpected connections from tests running in parallel.

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

PR Review: https://git.openjdk.org/jdk/pull/25844#pullrequestreview-2934464465
PR Review Comment: https://git.openjdk.org/jdk/pull/25844#discussion_r2151532528

Reply via email to