On Thu, 9 Oct 2025 15:54:02 GMT, Josiah Noel <[email protected]> wrote:
>> Following the guideline of the last comment on >> [JDK-8349670](https://bugs.openjdk.org/browse/JDK-8349670?focusedId=14794649&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14794649), >> resolves the issue where sending a 1xx status code would close the input >> stream, preventing the server from reading the body. >> >> - When a 1xx status code is sent by `sendResponseHeaders`, the input/output >> streams will not be closed prematurely. >> - sentHeaders will not be set to true when sending 1xx status codes >> - 100-continue will be sent automatically when trying to read the >> inputstream if `Expect: 100-continue` header is present > > Josiah Noel has updated the pull request incrementally with one additional > commit since the last revision: > > pr comment test/jdk/com/sun/net/httpserver/Send1xxResponsesTest.java line 62: > 60: @Test > 61: public void testAutoContinue() throws Exception { > 62: System.out.println("testContinue()"); Suggestion: System.out.println("testAutoContinue()"); test/jdk/com/sun/net/httpserver/Send1xxResponsesTest.java line 63: > 61: public void testAutoContinue() throws Exception { > 62: System.out.println("testContinue()"); > 63: InetAddress loopback = InetAddress.getLoopbackAddress(); Suggestion: InetAddress loopback = InetAddress.getLoopbackAddress(); String replyMsg = "Here is my reply!"; test/jdk/com/sun/net/httpserver/Send1xxResponsesTest.java line 70: > 68: msg -> { > 69: System.err.println("Handling request: " + > msg.getRequestURI()); > 70: byte[] reply = "Here is my reply!".getBytes(UTF_8); Suggestion: byte[] reply = replyMsg.getBytes(UTF_8); test/jdk/com/sun/net/httpserver/Send1xxResponsesTest.java line 74: > 72: msg.getRequestBody().readAllBytes(); > 73: msg.sendResponseHeaders(100, -1); > 74: msg.sendResponseHeaders(100, -1); Manually sending 100 is tested by the other test method. Suggestion: test/jdk/com/sun/net/httpserver/Send1xxResponsesTest.java line 85: > 83: System.out.println("Server started at port " + > server.getAddress().getPort()); > 84: > 85: runRawSocketHttpClient(loopback, > server.getAddress().getPort(), 0); Suggestion: runRawSocketHttpClient(loopback, server.getAddress().getPort(), true, replyMsg, 100, 200); test/jdk/com/sun/net/httpserver/Send1xxResponsesTest.java line 95: > 93: @Test > 94: public void testManualContinue() throws Exception { > 95: System.out.println("testAutoContinue()"); Suggestion: System.out.println("testManualContinue()"); test/jdk/com/sun/net/httpserver/Send1xxResponsesTest.java line 96: > 94: public void testManualContinue() throws Exception { > 95: System.out.println("testAutoContinue()"); > 96: InetAddress loopback = InetAddress.getLoopbackAddress(); Suggestion: InetAddress loopback = InetAddress.getLoopbackAddress(); String replyMsg = "Here is my other reply!"; test/jdk/com/sun/net/httpserver/Send1xxResponsesTest.java line 103: > 101: msg -> { > 102: System.err.println("Handling request: " + > msg.getRequestURI()); > 103: byte[] reply = "Here is my reply!".getBytes(UTF_8); Suggestion: byte[] reply = replyMsg.getBytes(UTF_8); test/jdk/com/sun/net/httpserver/Send1xxResponsesTest.java line 119: > 117: System.out.println("Server started at port " + > server.getAddress().getPort()); > 118: > 119: runRawSocketHttpClient(loopback, > server.getAddress().getPort(), 0); Suggestion: runRawSocketHttpClient(loopback, server.getAddress().getPort(), true, replyMsg, 100, 100, 100, 100, 200); test/jdk/com/sun/net/httpserver/Send1xxResponsesTest.java line 127: > 125: } > 126: > 127: static void runRawSocketHttpClient(InetAddress address, int port, > int contentLength) Suggestion: static void runRawSocketHttpClient(InetAddress address, int port, boolean exoectContinue, String expectedReply, int... epectedStatusCodes) test/jdk/com/sun/net/httpserver/Send1xxResponsesTest.java line 150: > 148: writer.print("Content-Length: " + contentLength + CRLF); > 149: writer.print("Connection: keep-alive" + CRLF); > 150: writer.print("Expect: 100-continue" + CRLF); Suggestion: if (expectContinue) { writer.print("Expect: 100-continue" + CRLF); } test/jdk/com/sun/net/httpserver/Send1xxResponsesTest.java line 171: > 169: else if (line.startsWith("HTTP/1.1 100")) { > 170: foundContinue = true; > 171: } Change that to build a list of received status codes test/jdk/com/sun/net/httpserver/Send1xxResponsesTest.java line 175: > 173: if (!foundThirdContinue) { > 174: throw new IOException("Did not receive two 100 continue > from server"); > 175: } Move that to after having received the final staus code, verifying that the received codes correspond to what was passed in parameter. test/jdk/com/sun/net/httpserver/Send1xxResponsesTest.java line 186: > 184: break; > 185: } > 186: System.out.println("final response \"" + line + "\""); add the final code to the list and verify the list matches. Read the body and verify it matches too. test/jdk/com/sun/net/httpserver/Send1xxResponsesTest.java line 191: > 189: } finally { > 190: // give time to the server to try & drain its input stream > 191: Thread.sleep(500); Not sure why we need that sleep? test/jdk/com/sun/net/httpserver/Send1xxResponsesTest.java line 199: > 197: // give time to the server to trigger its assertion > 198: // error before closing the connection > 199: Thread.sleep(500); Which assertion error is that? test/jdk/com/sun/net/httpserver/Send1xxResponsesTest.java line 216: > 214: System.out.println("Client finished."); > 215: } > 216: } Please add new line at the end. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27069#discussion_r2417230889 PR Review Comment: https://git.openjdk.org/jdk/pull/27069#discussion_r2417248852 PR Review Comment: https://git.openjdk.org/jdk/pull/27069#discussion_r2417249814 PR Review Comment: https://git.openjdk.org/jdk/pull/27069#discussion_r2417234936 PR Review Comment: https://git.openjdk.org/jdk/pull/27069#discussion_r2417251104 PR Review Comment: https://git.openjdk.org/jdk/pull/27069#discussion_r2417253981 PR Review Comment: https://git.openjdk.org/jdk/pull/27069#discussion_r2417256808 PR Review Comment: https://git.openjdk.org/jdk/pull/27069#discussion_r2417257669 PR Review Comment: https://git.openjdk.org/jdk/pull/27069#discussion_r2417265085 PR Review Comment: https://git.openjdk.org/jdk/pull/27069#discussion_r2417270204 PR Review Comment: https://git.openjdk.org/jdk/pull/27069#discussion_r2417274147 PR Review Comment: https://git.openjdk.org/jdk/pull/27069#discussion_r2417282180 PR Review Comment: https://git.openjdk.org/jdk/pull/27069#discussion_r2417289995 PR Review Comment: https://git.openjdk.org/jdk/pull/27069#discussion_r2417301310 PR Review Comment: https://git.openjdk.org/jdk/pull/27069#discussion_r2417302346 PR Review Comment: https://git.openjdk.org/jdk/pull/27069#discussion_r2417304703 PR Review Comment: https://git.openjdk.org/jdk/pull/27069#discussion_r2417305611
