On Wed, 18 Jun 2025 01:51:36 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 one additional > commit since the last revision: > > Daniel's suggestion - use trailing slash for path registered for the handler Hello Michael, I see that you already found out what caused the regression, but yes it's the `if` block check which went wrong in our clean up. The `checkSetHost()` method which was being used previously would always return `true` when SecurityManager wasn't set and that meant that we wouldn't enter the `if` block when the `requestHost` was non-null. That check broke after our clean up and is now fixed in this PR. ------------- PR Comment: https://git.openjdk.org/jdk/pull/25844#issuecomment-2983249755
