Author: markt Date: Thu Aug 30 21:58:25 2012 New Revision: 1379180 URL: http://svn.apache.org/viewvc?rev=1379180&view=rev Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53677 Ensure a 500 response of the HTTP headers exceed the size limit
Added: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/HeadersTooLargeException.java - copied unchanged from r1379178, tomcat/trunk/java/org/apache/coyote/http11/HeadersTooLargeException.java Modified: tomcat/tc7.0.x/trunk/ (props changed) tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java Propchange: tomcat/tc7.0.x/trunk/ ------------------------------------------------------------------------------ Merged /tomcat/trunk:r1379178 Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1379180&r1=1379179&r2=1379180&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Thu Aug 30 21:58:25 2012 @@ -761,12 +761,21 @@ public abstract class AbstractHttp11Proc // Validate and write response headers try { - prepareResponse(); +// try { + prepareResponse(); +// } catch (IllegalStateException e) { + // Headers too big. Likely too late to do anything about it +// response.reset(); +// response.setStatus(500); +// response.setHeader("Connection", "close"); +// response.sendHeaders(); +// } getOutputBuffer().commit(); } catch (IOException e) { // Set error flag error = true; } + } else if (actionCode == ActionCode.ACK) { // Acknowledge request // Send a 100 status back if it makes sense (response not committed @@ -1012,6 +1021,15 @@ public abstract class AbstractHttp11Proc setCometTimeouts(socketWrapper); } catch (InterruptedIOException e) { error = true; + } catch (HeadersTooLargeException e) { + error = true; + // The response should not have been committed but check it + // anyway to be safe + if (!response.isCommitted()) { + response.reset(); + response.setStatus(500); + response.setHeader("Connection", "close"); + } } catch (Throwable t) { ExceptionUtils.handleThrowable(t); getLog().error(sm.getString( Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java?rev=1379180&r1=1379179&r2=1379180&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java Thu Aug 30 21:58:25 2012 @@ -250,7 +250,10 @@ public abstract class AbstractOutputBuff // Recycle Request object response.recycle(); - + // These will need to be reset if the reset was triggered by the error + // handling if the headers were too large + pos = 0; + byteCount = 0; } /** @@ -538,10 +541,9 @@ public abstract class AbstractOutputBuff * Checks to see if there is enough space in the buffer to write the * requested number of bytes. */ - private void checkLengthBeforeWrite(int length) - throws IllegalStateException { + private void checkLengthBeforeWrite(int length) { if (pos + length > buf.length) { - throw new IllegalStateException( + throw new HeadersTooLargeException( sm.getString("iob.responseheadertoolarge.error")); } } Modified: tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java?rev=1379180&r1=1379179&r2=1379180&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java (original) +++ tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java Thu Aug 30 21:58:25 2012 @@ -18,6 +18,7 @@ package org.apache.coyote.http11; import java.io.File; import java.io.IOException; +import java.nio.CharBuffer; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -311,6 +312,68 @@ public class TestAbstractHttp11Processor assertEquals("OK", responseBody.toString()); } + @Test + public void testBug53677a() throws Exception { + doTestBug53677(false); + } + + @Test + public void testBug53677b() throws Exception { + doTestBug53677(true); + } + + private void doTestBug53677(boolean flush) throws Exception { + Tomcat tomcat = getTomcatInstance(); + + // Must have a real docBase - just use temp + Context ctxt = tomcat.addContext("", + System.getProperty("java.io.tmpdir")); + + Tomcat.addServlet(ctxt, "LargeHeaderServlet", + new LargeHeaderServlet(flush)); + ctxt.addServletMapping("/test", "LargeHeaderServlet"); + + tomcat.start(); + + ByteChunk responseBody = new ByteChunk(); + Map<String,List<String>> responseHeaders = + new HashMap<String,List<String>>(); + int rc = getUrl("http://localhost:" + getPort() + "/test", responseBody, + responseHeaders); + + assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, rc); + if (responseBody.getLength() > 0) { + // It will be >0 if the standard error page handlign has been + // triggered + assertFalse(responseBody.toString().contains("FAIL")); + } + } + + private static final class LargeHeaderServlet extends HttpServlet { + + private static final long serialVersionUID = 1L; + + boolean flush = false; + + public LargeHeaderServlet(boolean flush) { + this.flush = flush; + } + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + String largeValue = + CharBuffer.allocate(10000).toString().replace('\0', 'x'); + resp.setHeader("x-Test", largeValue); + if (flush) { + resp.flushBuffer(); + } + resp.setContentType("text/plain"); + resp.getWriter().print("FAIL"); + } + + } + // flushes with no content-length set // should result in chunking on HTTP 1.1 private static final class NoContentLengthFlushingServlet --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org