On Fri, 27 Mar 2026 12:28:55 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. > > Jaikiran Pai has updated the pull request incrementally with two additional > commits since the last revision: > > - add additional bad status lines > - Daniel's review - minor cleanup to the test src/java.base/share/classes/sun/net/www/protocol/http/HttpURLConnection.java line 2040: > 2038: // parsed. > 2039: private static int parseConnectResponseCode(final String > statusLine) { > 2040: final int invalidStatusLine = 0; *Nit:* We generally use `-1` to indicate a failure while reading the status line. You can search for usages of `-1` in this file, in particular, see lines 1076 and 1415. test/jdk/java/net/HttpURLConnection/ProxyBadStatusLine.java line 216: > 214: } > 215: > 216: private static byte[] readRequest(InputStream is) throws > IOException { I've two remarks regarding this method: 1. It doesn't handle the case when double-CRLF is not found, yet EOF is reached. 2. It can use some diet using `Arrays::mismatch` and `::copyOfRange`: private static byte[] readRequest(InputStream is) throws IOException { // we don't expect the HTTP request body in this test to be larger than this size final byte[] buff = new byte[4096]; int numRead = 0; boolean found = false; byte[] delimiter = {'\r', '\n', '\r', '\n'}; for (int c; !found && (c = is.read()) != -1 && numRead < buff.length;) { buff[numRead++] = (byte) c; if (numRead >= delimiter.length) { found = Arrays.mismatch(buff, numRead - delimiter.length, numRead, delimiter, 0, delimiter.length) < 0; } } if (!found) { throw new IOException( "Couldn't locate double-CRLF in request: " + new String(buff, 0, numRead, ISO_8859_1)); } return Arrays.copyOfRange(buff, 0, numRead); } ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30466#discussion_r3008446784 PR Review Comment: https://git.openjdk.org/jdk/pull/30466#discussion_r3008941896
