RFR: 8373677: Clear text HttpServer connection could fail fast if receiving SSL ClientHello [v4]
Daniel Fuchs
dfuchs at openjdk.org
Wed Dec 17 12:17:03 UTC 2025
On Wed, 17 Dec 2025 12:02:18 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:
>> Daniel Fuchs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 10 additional commits since the last revision:
>>
>> - .toString() is not needed
>> - Review feedback: improved logging
>> - Merge branch 'master' into ClearTextSSL-8373677
>> - Update test/jdk/com/sun/net/httpserver/ClearTextServerSSL.java
>>
>> Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>
>> - Update src/jdk.httpserver/share/classes/sun/net/httpserver/Request.java
>>
>> Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>
>> - minor test fix - unused import + obsolete comment
>> - fix whitespace
>> - fix copyright year in test
>> - add bug id to test
>> - 8373677: Clear text HttpServer connection could fail fast if receiving SSL ClientHello
>
> src/jdk.httpserver/share/classes/sun/net/httpserver/Request.java line 119:
>
>> 117: throw new ProtocolException("Unexpected start of request line");
>> 118: }
>> 119: offset++;
>
> The changes look good to me. It took me a while to understand what this `offset` was and how those increments are used. It looks like it behaves merely like a boolean to decide whether or not to check the byte against the `FIRST_CHAR`. Maybe we could change `int offset` to `boolean firstByteChecked = false;`. It's also OK if you like to leave it in the current form.
I'll leave the offset. I have limited the check to the first byte, but arguably we could improve the parsing of the request line in the future by checking the full syntax here. It would be a more complex enhancement so better be handled in a future PR.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28827#discussion_r2626820612
More information about the net-dev
mailing list