Bug 68228 - Status code can no longer be set after a read exception occurs in 9.0.83 or later
Summary: Status code can no longer be set after a read exception occurs in 9.0.83 or l...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 9
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 9.0.83
Hardware: PC Mac OS X 10.1
: P2 normal (vote)
Target Milestone: -----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-11-26 23:50 UTC by adwsingh
Modified: 2023-12-04 13:41 UTC (History)
2 users (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description adwsingh 2023-11-26 23:50:27 UTC
This is similar to https://bz.apache.org/bugzilla/show_bug.cgi?id=68037, but now even the sync method fails after upgrading to 9.0.83.

Here is the test that succeeds with 9.0.82 and fails with 9.0.83.

import org.apache.catalina.Context;
import org.apache.catalina.LifecycleException;
import org.apache.catalina.connector.ClientAbortException;
import org.apache.catalina.connector.Connector;
import org.apache.catalina.core.StandardHost;
import org.apache.catalina.startup.Tomcat;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.BufferedReader;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.net.Socket;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;

public class ReproducibleTest {

    static Tomcat tomcat;

    @BeforeAll
    static void setup() throws LifecycleException {
        tomcat = new Tomcat();
        ExecutorService executorService = Executors.newFixedThreadPool(5);
        Context ctx = tomcat.addContext("", new File(".").getAbsolutePath());
        Tomcat.addServlet(ctx, "TestServlet", new SyncServlet());
        StandardHost host = (StandardHost) tomcat.getHost();
        host.setErrorReportValveClass(null);
        Connector connector = new Connector();
        connector.setProperty("address", "http://localhost");
        connector.setPort(8000);
        connector.setProperty("connectionTimeout", String.valueOf(100));
        connector.getProtocolHandler().setExecutor(executorService);
        tomcat.getService().addConnector(connector);
        ctx.addServletMappingDecoded("/*", "TestServlet");
        tomcat.start();
    }

    @AfterAll
    static void destroy() throws LifecycleException {
        tomcat.stop();
        tomcat.destroy();
    }

    @Test
    void testTimeoutGets408() throws LifecycleException, IOException {
        try (Socket s = new Socket("localhost", 8000)) {
            String request = "GET /async HTTP/1.1\r\nHost: localhost\r\ncontent-length: 101\r\n\r\n";
            sendBadRequest(s, request, 408);
        }

    }

    private static void sendBadRequest(Socket socket, String request, int expectedStatusCode) throws IOException {
        OutputStream os = socket.getOutputStream();
        os.write(request.getBytes(UTF_8));
        InputStream is = socket.getInputStream();
        BufferedReader reader = new BufferedReader(new InputStreamReader(is, UTF_8));
        String opening = reader.readLine();
        assertNotNull(opening, "Didn't get back a response");
        StringBuilder sb = new StringBuilder(opening);

        try {
            assertTrue(opening.startsWith("HTTP/1.1 " + expectedStatusCode), "expected status code " + expectedStatusCode + " but got " + opening);
            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;
                }

                assertFalse(line.contains("Exception Report"));
                assertFalse(line.contains("Status Report"));
            }

            assertTrue(connectionClose, "No 'Connection: close' header seen");
        } catch (Throwable t) {
            fail("Response:\n" + sb, t);
        }
    }

    static final class SyncServlet extends HttpServlet {
        @Override
        protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
            try {
                while (req.getInputStream().read() != -1) ;
                resp.setStatus(200);
                resp.flushBuffer();
            } catch (ClientAbortException e) {
                resp.sendError(408);
            }
        }
    }

}
Comment 1 Mark Thomas 2023-11-30 09:13:48 UTC
I suspect the fix for CVE-2023-46589 was responsible for this change.

I'll take another look and see if there is a way to get Tomcat to use 408 for a read timeout.
Comment 2 Mark Thomas 2023-12-04 13:41:37 UTC
Fixed in:
- 11.0.x for 11.0.0-M15 onwards
- 10.1.x for 10.1.17 onwards
-  9.0.x for  9.0.84 onwards
-  8.5.x for  8.5.97 onwards

You won't have complete control over the status code but a timeout will now result in a 408 response to the client.