On Fri, 27 Mar 2026 06:56:17 GMT, Jaikiran Pai <[email protected]> wrote:
> Can I please get a review of this change which proposes to address the issue > noted in https://bugs.openjdk.org/browse/JDK-8373778? > > When a CONNECT request to a proxy returns an invalid status line, then the > current code in the internal implementation of `HttpURLConnection` runs into > an exception trying to parse that status line. That exception propagates as > an unspecified `java.util.NoSuchElementException` to the application. > > The commit in this PR addresses it by doing additional checks on the status > line to ensure it is valid and if it isn't then it raises an `IOException`. > This is the same `IOException` that would have been raised for a few other > invalid responses for a CONNECT request. > > A new jtreg test has been introduced to reproduce the issue and verify the > fix. tier1, tier2, tier3 testing succeeded with this change. The code changes look reasonable. Some minor comments on the test. test/jdk/java/net/HttpURLConnection/ProxyBadStatusLine.java line 54: > 52: > 53: static List<String> badStatusLines() { > 54: return List.of("HTTP/1.", should we add a blank line here too? test/jdk/java/net/HttpURLConnection/ProxyBadStatusLine.java line 107: > 105: private static final class BadProxyServer implements Runnable, > AutoCloseable { > 106: private static final int CR = 13; > 107: private static final int LF = 10; Any reason to not initialize the int constant with the corresponding char value? Suggestion: private static final int CR = '\r'; private static final int LF = '\n'; test/jdk/java/net/HttpURLConnection/ProxyBadStatusLine.java line 207: > 205: } > 206: > 207: private static byte[] readRequestLine(InputStream is) throws > IOException { So this does not simply read the request line, but the full request headers block, up to 4k. Maybe the name of the method should reflect that. ------------- PR Review: https://git.openjdk.org/jdk/pull/30466#pullrequestreview-4019952031 PR Review Comment: https://git.openjdk.org/jdk/pull/30466#discussion_r3000040359 PR Review Comment: https://git.openjdk.org/jdk/pull/30466#discussion_r2999926148 PR Review Comment: https://git.openjdk.org/jdk/pull/30466#discussion_r2999946200
