This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push: new 12ea1041c1 Fix 68228 - return a 408 after a read timeout 12ea1041c1 is described below commit 12ea1041c1438ba688de04885316957f4a353c94 Author: Mark Thomas <ma...@apache.org> AuthorDate: Mon Dec 4 11:06:17 2023 +0000 Fix 68228 - return a 408 after a read timeout This isn't a complete fix as it doesn't allow the user to control the status code after an IOException during read. --- .../org/apache/catalina/connector/InputBuffer.java | 8 +- .../catalina/connector/TestClientReadTimeout.java | 123 +++++++++++++++++++++ webapps/docs/changelog.xml | 5 + 3 files changed, 135 insertions(+), 1 deletion(-) diff --git a/java/org/apache/catalina/connector/InputBuffer.java b/java/org/apache/catalina/connector/InputBuffer.java index 2df4e027f0..074ed29a01 100644 --- a/java/org/apache/catalina/connector/InputBuffer.java +++ b/java/org/apache/catalina/connector/InputBuffer.java @@ -18,6 +18,7 @@ package org.apache.catalina.connector; import java.io.IOException; import java.io.Reader; +import java.net.SocketTimeoutException; import java.nio.Buffer; import java.nio.ByteBuffer; import java.nio.CharBuffer; @@ -30,6 +31,7 @@ import java.util.concurrent.ConcurrentMap; import javax.servlet.ReadListener; import javax.servlet.RequestDispatcher; +import javax.servlet.http.HttpServletResponse; import org.apache.catalina.security.SecurityUtil; import org.apache.coyote.ActionCode; @@ -337,7 +339,11 @@ public class InputBuffer extends Reader implements ByteChunk.ByteInputChannel, A Request request = (Request) coyoteRequest.getNote(CoyoteAdapter.ADAPTER_NOTES); Response response = request.getResponse(); request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, e); - response.sendError(400); + if (e instanceof SocketTimeoutException) { + response.sendError(HttpServletResponse.SC_REQUEST_TIMEOUT); + } else { + response.sendError(HttpServletResponse.SC_BAD_REQUEST); + } } diff --git a/test/org/apache/catalina/connector/TestClientReadTimeout.java b/test/org/apache/catalina/connector/TestClientReadTimeout.java new file mode 100644 index 0000000000..4b90bff6de --- /dev/null +++ b/test/org/apache/catalina/connector/TestClientReadTimeout.java @@ -0,0 +1,123 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.catalina.connector; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.OutputStream; +import java.net.Socket; +import java.nio.charset.StandardCharsets; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.Context; +import org.apache.catalina.LifecycleException; +import org.apache.catalina.core.StandardHost; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; + + +public class TestClientReadTimeout extends TomcatBaseTest { + + static Tomcat tomcat; + + @Test + public void testTimeoutGets408() throws IOException, LifecycleException { + // Setup Tomcat instance + Tomcat tomcat = getTomcatInstance(); + + // No file system docBase required + Context ctx = tomcat.addContext("", null); + + ((StandardHost) tomcat.getHost()).setErrorReportValveClass(null); + + Tomcat.addServlet(ctx, "TestServlet", new SyncServlet()); + ctx.addServletMappingDecoded("/*", "TestServlet"); + + tomcat.start(); + + try (Socket socket = new Socket("localhost", getPort())) { + String request = "GET /async HTTP/1.1\r\nHost: localhost\r\ncontent-length: 101\r\n\r\n"; + + OutputStream os = socket.getOutputStream(); + os.write(request.getBytes(StandardCharsets.UTF_8)); + InputStream is = socket.getInputStream(); + BufferedReader reader = new BufferedReader(new InputStreamReader(is, StandardCharsets.UTF_8)); + String opening = reader.readLine(); + if (tomcat.getConnector().getProtocolHandlerClassName().contains("Nio2")) { + Assert.assertNull("NIO2 unexpectedly returned a response", opening); + } else { + Assert.assertNotNull("Didn't get back a response", opening); + StringBuilder sb = new StringBuilder(opening); + + try { + Assert.assertTrue( + "expected status code " + HttpServletResponse.SC_REQUEST_TIMEOUT + " but got " + opening, + opening.startsWith("HTTP/1.1 " + HttpServletResponse.SC_REQUEST_TIMEOUT)); + boolean connectionClose = false; + while (reader.ready()) { + String line = reader.readLine(); + if (line == null) { + break; + } + + sb.append("\n").append(line); + if ("connection: close".equalsIgnoreCase(line)) { + connectionClose = true; + } + + Assert.assertFalse(line.contains("Exception Report")); + Assert.assertFalse(line.contains("Status Report")); + } + + Assert.assertTrue("No 'Connection: close' header seen", connectionClose); + } catch (Throwable t) { + Assert.fail("Response:\n" + sb); + t.printStackTrace(); + } + } + } + } + + + static final class SyncServlet extends HttpServlet { + + private static final long serialVersionUID = 1L; + + @Override + protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + try { + while (req.getInputStream().read() != -1) { + // NO-OP - Any data read is ignored + } + resp.setStatus(200); + resp.flushBuffer(); + } catch (ClientAbortException e) { + // resp.sendError(408); + } + } + } + +} \ No newline at end of file diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index f7640257ff..12ee52195c 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -121,6 +121,11 @@ called if <code>AsyncListener.onError()</code> calls <code>AsyncContext.dispatch()</code>. (markt) </fix> + <fix> + <bug>68228</bug>: Use a 408 status code if a read timeout occurs during + HTTP request processing. Includes a test case based on code provided by + adwsingh. (markt) + </fix> </changelog> </subsection> <subsection name="Jasper"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org