On Tue, 17 Jun 2025 13:53:15 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.
>
> Jaikiran Pai has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Volkan's suggestion - use test specific context root for the handler
>  - Daniel's suggestion - use setIfNotSet

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

> 58: class HostHeaderTest {
> 59: 
> 60:     private static final String SERVER_CTX_ROOT = "/8359709";

Suggestion:

    private static final String SERVER_CTX_ROOT = "/8359709/";

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25844#discussion_r2152391670

Reply via email to