Index: apache-tomcat-9.0.82-src/java/org/apache/catalina/connector/InputBuffer.java =================================================================== --- apache-tomcat-9.0.82-src.orig/java/org/apache/catalina/connector/InputBuffer.java +++ apache-tomcat-9.0.82-src/java/org/apache/catalina/connector/InputBuffer.java @@ -29,6 +29,7 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import javax.servlet.ReadListener; +import javax.servlet.RequestDispatcher; import org.apache.catalina.security.SecurityUtil; import org.apache.coyote.ActionCode; @@ -295,6 +296,7 @@ public class InputBuffer extends Reader * * @throws IOException An underlying IOException occurred */ + @SuppressWarnings("deprecation") @Override public int realReadBytes() throws IOException { if (closed) { @@ -307,10 +309,24 @@ public class InputBuffer extends Reader try { return coyoteRequest.doRead(this); + } catch (BadRequestException bre) { + // Set flag used by asynchronous processing to detect errors on non-container threads + coyoteRequest.setErrorException(bre); + // In synchronous processing, this exception may be swallowed by the application so set error flags here. + coyoteRequest.setAttribute(RequestDispatcher.ERROR_EXCEPTION, bre); + coyoteRequest.getResponse().setStatus(400); + coyoteRequest.getResponse().setError(); + // Make the exception visible to the application + throw bre; } catch (IOException ioe) { + // Set flag used by asynchronous processing to detect errors on non-container threads coyoteRequest.setErrorException(ioe); - // An IOException on a read is almost always due to - // the remote client aborting the request. + // In synchronous processing, this exception may be swallowed by the application so set error flags here. + coyoteRequest.setAttribute(RequestDispatcher.ERROR_EXCEPTION, ioe); + coyoteRequest.getResponse().setStatus(400); + coyoteRequest.getResponse().setError(); + // Any other IOException on a read is almost always due to the remote client aborting the request. + // Make the exception visible to the application throw new ClientAbortException(ioe); } } Index: apache-tomcat-9.0.82-src/java/org/apache/catalina/connector/ClientAbortException.java =================================================================== --- apache-tomcat-9.0.82-src.orig/java/org/apache/catalina/connector/ClientAbortException.java +++ apache-tomcat-9.0.82-src/java/org/apache/catalina/connector/ClientAbortException.java @@ -16,14 +16,12 @@ */ package org.apache.catalina.connector; -import java.io.IOException; - /** * Extend IOException to identify it as being caused by an abort of a request by a remote client. * * @author Glenn L. Nielsen */ -public final class ClientAbortException extends IOException { +public final class ClientAbortException extends BadRequestException { private static final long serialVersionUID = 1L; Index: apache-tomcat-9.0.82-src/java/org/apache/catalina/core/ApplicationDispatcher.java =================================================================== --- apache-tomcat-9.0.82-src.orig/java/org/apache/catalina/core/ApplicationDispatcher.java +++ apache-tomcat-9.0.82-src/java/org/apache/catalina/core/ApplicationDispatcher.java @@ -41,7 +41,7 @@ import org.apache.catalina.AsyncDispatch import org.apache.catalina.Context; import org.apache.catalina.Globals; import org.apache.catalina.Wrapper; -import org.apache.catalina.connector.ClientAbortException; +import org.apache.catalina.connector.BadRequestException; import org.apache.catalina.connector.Request; import org.apache.catalina.connector.RequestFacade; import org.apache.catalina.connector.Response; @@ -661,7 +661,7 @@ final class ApplicationDispatcher implem filterChain.doFilter(request, response); } // Servlet Service Method is called by the FilterChain - } catch (ClientAbortException e) { + } catch (BadRequestException e) { ioException = e; } catch (IOException e) { wrapper.getLogger().error(sm.getString("applicationDispatcher.serviceException", wrapper.getName()), e); @@ -672,7 +672,7 @@ final class ApplicationDispatcher implem wrapper.unavailable(e); } catch (ServletException e) { Throwable rootCause = StandardWrapper.getRootCause(e); - if (!(rootCause instanceof ClientAbortException)) { + if (!(rootCause instanceof BadRequestException)) { wrapper.getLogger().error(sm.getString("applicationDispatcher.serviceException", wrapper.getName()), rootCause); } Index: apache-tomcat-9.0.82-src/java/org/apache/catalina/core/StandardWrapperValve.java =================================================================== --- apache-tomcat-9.0.82-src.orig/java/org/apache/catalina/core/StandardWrapperValve.java +++ apache-tomcat-9.0.82-src/java/org/apache/catalina/core/StandardWrapperValve.java @@ -32,7 +32,7 @@ import org.apache.catalina.Container; import org.apache.catalina.Context; import org.apache.catalina.Globals; import org.apache.catalina.LifecycleException; -import org.apache.catalina.connector.ClientAbortException; +import org.apache.catalina.connector.BadRequestException; import org.apache.catalina.connector.Request; import org.apache.catalina.connector.Response; import org.apache.catalina.valves.ValveBase; @@ -170,7 +170,7 @@ final class StandardWrapperValve extends } } - } catch (ClientAbortException | CloseNowException e) { + } catch (BadRequestException | CloseNowException e) { if (container.getLogger().isDebugEnabled()) { container.getLogger().debug( sm.getString("standardWrapper.serviceException", wrapper.getName(), context.getName()), e); @@ -191,7 +191,7 @@ final class StandardWrapperValve extends // do not want to do exception(request, response, e) processing } catch (ServletException e) { Throwable rootCause = StandardWrapper.getRootCause(e); - if (!(rootCause instanceof ClientAbortException)) { + if (!(rootCause instanceof BadRequestException)) { container.getLogger().error(sm.getString("standardWrapper.serviceExceptionRoot", wrapper.getName(), context.getName(), e.getMessage()), rootCause); } Index: apache-tomcat-9.0.82-src/java/org/apache/catalina/connector/BadRequestException.java =================================================================== --- /dev/null +++ apache-tomcat-9.0.82-src/java/org/apache/catalina/connector/BadRequestException.java @@ -0,0 +1,68 @@ +/* + * 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.IOException; + +/** + * Extend IOException to identify it as being caused by a bad request from a remote client. + */ +public class BadRequestException extends IOException { + + private static final long serialVersionUID = 1L; + + + // ------------------------------------------------------------ Constructors + + /** + * Construct a new BadRequestException with no other information. + */ + public BadRequestException() { + super(); + } + + + /** + * Construct a new BadRequestException for the specified message. + * + * @param message Message describing this exception + */ + public BadRequestException(String message) { + super(message); + } + + + /** + * Construct a new BadRequestException for the specified throwable. + * + * @param throwable Throwable that caused this exception + */ + public BadRequestException(Throwable throwable) { + super(throwable); + } + + + /** + * Construct a new BadRequestException for the specified message and throwable. + * + * @param message Message describing this exception + * @param throwable Throwable that caused this exception + */ + public BadRequestException(String message, Throwable throwable) { + super(message, throwable); + } +} Index: apache-tomcat-9.0.82-src/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java =================================================================== --- apache-tomcat-9.0.82-src.orig/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java +++ apache-tomcat-9.0.82-src/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java @@ -428,6 +428,83 @@ public class TestChunkedInputFilter exte } } + + @Test + public void testTrailerHeaderNameNotTokenThrowException() throws Exception { + doTestTrailerHeaderNameNotToken(false); + } + + @Test + public void testTrailerHeaderNameNotTokenSwallowException() throws Exception { + doTestTrailerHeaderNameNotToken(true); + } + + private void doTestTrailerHeaderNameNotToken(boolean swallowException) throws Exception { + + // Setup Tomcat instance + Tomcat tomcat = getTomcatInstance(); + + // No file system docBase required + Context ctx = tomcat.addContext("", null); + + Tomcat.addServlet(ctx, "servlet", new SwallowBodyServlet(swallowException)); + ctx.addServletMappingDecoded("/", "servlet"); + + tomcat.start(); + + String[] request = new String[]{ + "POST / HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost" + SimpleHttpClient.CRLF + + "Transfer-encoding: chunked" + SimpleHttpClient.CRLF + + "Content-Type: application/x-www-form-urlencoded" + SimpleHttpClient.CRLF + + "Connection: close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF + + "3" + SimpleHttpClient.CRLF + + "a=0" + SimpleHttpClient.CRLF + + "4" + SimpleHttpClient.CRLF + + "&b=1" + SimpleHttpClient.CRLF + + "0" + SimpleHttpClient.CRLF + + "x@trailer: Test" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF }; + + TrailerClient client = new TrailerClient(tomcat.getConnector().getLocalPort()); + client.setRequest(request); + + client.connect(); + client.processRequest(); + // Expected to fail because of invalid trailer header name + Assert.assertTrue(client.getResponseLine(), client.isResponse400()); + } + + private static class SwallowBodyServlet extends HttpServlet { + private static final long serialVersionUID = 1L; + + private final boolean swallowException; + + SwallowBodyServlet(boolean swallowException) { + this.swallowException = swallowException; + } + + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + resp.setContentType("text/plain"); + PrintWriter pw = resp.getWriter(); + + // Read the body + InputStream is = req.getInputStream(); + try { + while (is.read() > -1) { + } + pw.write("OK"); + } catch (IOException ioe) { + if (!swallowException) { + throw ioe; + } + } + } + } + private static class EchoHeaderServlet extends HttpServlet { private static final long serialVersionUID = 1L; Index: apache-tomcat-9.0.82-src/webapps/docs/changelog.xml =================================================================== --- apache-tomcat-9.0.82-src.orig/webapps/docs/changelog.xml +++ apache-tomcat-9.0.82-src/webapps/docs/changelog.xml @@ -120,7 +120,7 @@ use of fully qualified class names in 9.0.81 that broke the jdbc-pool. (markt) - +
@@ -148,6 +148,11 @@ Improve handling of failures within recycle() methods. (markt) + + Ensure that an IOException during the reading of the + request triggers always error handling, regardless of whether the + application swallows the exception. (markt) +