On Wed, 17 Dec 2025 12:02:18 GMT, Jaikiran Pai <[email protected]> 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 <[email protected]> >> - Update src/jdk.httpserver/share/classes/sun/net/httpserver/Request.java >> >> Co-authored-by: Andrey Turbanov <[email protected]> >> - 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
