Author: markt Date: Wed Apr 12 14:25:50 2017 New Revision: 1791124 URL: http://svn.apache.org/viewvc?rev=1791124&view=rev Log: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=60970 Fix infinite loop if application tries to write a large header to the response when using HTTP/2.
Added: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2UpgradeHandler.java (with props) Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java?rev=1791124&r1=1791123&r2=1791124&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java (original) +++ tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java Wed Apr 12 14:25:50 2017 @@ -546,29 +546,36 @@ class Http2UpgradeHandler extends Abstra while (state != State.COMPLETE) { state = getHpackEncoder().encode(coyoteResponse.getMimeHeaders(), target); target.flip(); - ByteUtil.setThreeBytes(header, 0, target.limit()); - if (first) { - first = false; - header[3] = FrameType.HEADERS.getIdByte(); - if (stream.getOutputBuffer().hasNoBody()) { - header[4] = FLAG_END_OF_STREAM; + if (state == State.COMPLETE || target.limit() > 0) { + ByteUtil.setThreeBytes(header, 0, target.limit()); + if (first) { + first = false; + header[3] = FrameType.HEADERS.getIdByte(); + if (stream.getOutputBuffer().hasNoBody()) { + header[4] = FLAG_END_OF_STREAM; + } + } else { + header[3] = FrameType.CONTINUATION.getIdByte(); + } + if (state == State.COMPLETE) { + header[4] += FLAG_END_OF_HEADERS; + } + if (log.isDebugEnabled()) { + log.debug(target.limit() + " bytes"); + } + ByteUtil.set31Bits(header, 5, stream.getIdentifier().intValue()); + try { + socketWrapper.write(true, header, 0, header.length); + socketWrapper.write(true, target); + socketWrapper.flush(true); + } catch (IOException ioe) { + handleAppInitiatedIOException(ioe); } - } else { - header[3] = FrameType.CONTINUATION.getIdByte(); - } - if (state == State.COMPLETE) { - header[4] += FLAG_END_OF_HEADERS; - } - if (log.isDebugEnabled()) { - log.debug(target.limit() + " bytes"); } - ByteUtil.set31Bits(header, 5, stream.getIdentifier().intValue()); - try { - socketWrapper.write(true, header, 0, header.length); - socketWrapper.write(true, target); - socketWrapper.flush(true); - } catch (IOException ioe) { - handleAppInitiatedIOException(ioe); + if (state == State.UNDERFLOW) { + target = ByteBuffer.allocate(target.capacity() * 2); + } else { + target.clear(); } } } Modified: tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java?rev=1791124&r1=1791123&r2=1791124&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java (original) +++ tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java Wed Apr 12 14:25:50 2017 @@ -27,6 +27,7 @@ import java.nio.charset.StandardCharsets import java.util.ArrayList; import java.util.List; import java.util.Locale; +import java.util.Random; import javax.net.SocketFactory; import javax.servlet.ServletException; @@ -59,6 +60,8 @@ public abstract class Http2TestBase exte // response now included a date header protected static final String DEFAULT_DATE = "Wed, 11 Nov 2015 19:18:42 GMT"; + private static final String HEADER_IGNORED = "x-ignore"; + static final String DEFAULT_CONNECTION_HEADER_VALUE = "Upgrade, HTTP2-Settings"; private static final byte[] EMPTY_SETTINGS_FRAME = { 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00 }; @@ -918,7 +921,12 @@ public abstract class Http2TestBase exte if ("date".equals(name)) { value = DEFAULT_DATE; } - trace.append(lastStreamId + "-Header-[" + name + "]-[" + value + "]\n"); + // Some header values vary so ignore them + if (HEADER_IGNORED.equals(name)) { + trace.append(lastStreamId + "-Header-[" + name + "]-[...]\n"); + } else { + trace.append(lastStreamId + "-Header-[" + name + "]-[" + value + "]\n"); + } } @@ -1131,6 +1139,26 @@ public abstract class Http2TestBase exte } } + + static class LargeHeaderServlet extends HttpServlet { + + private static final long serialVersionUID = 1L; + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + resp.setContentType("text/plain"); + resp.setCharacterEncoding("UTF-8"); + StringBuilder headerValue = new StringBuilder(); + Random random = new Random(); + while (headerValue.length() < 2048) { + headerValue.append(Long.toString(random.nextLong())); + } + resp.setHeader(HEADER_IGNORED, headerValue.toString()); + resp.getWriter().print("OK"); + } + } + static class SettingValue { private final int setting; Added: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2UpgradeHandler.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/TestHttp2UpgradeHandler.java?rev=1791124&view=auto ============================================================================== --- tomcat/trunk/test/org/apache/coyote/http2/TestHttp2UpgradeHandler.java (added) +++ tomcat/trunk/test/org/apache/coyote/http2/TestHttp2UpgradeHandler.java Wed Apr 12 14:25:50 2017 @@ -0,0 +1,71 @@ +/* + * 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.coyote.http2; + +import java.nio.ByteBuffer; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.Context; +import org.apache.catalina.startup.Tomcat; + +public class TestHttp2UpgradeHandler extends Http2TestBase { + + // https://bz.apache.org/bugzilla/show_bug.cgi?id=60970 + @Test + public void testLargeHeader() throws Exception { + enableHttp2(); + + Tomcat tomcat = getTomcatInstance(); + + Context ctxt = tomcat.addContext("", null); + Tomcat.addServlet(ctxt, "simple", new SimpleServlet()); + ctxt.addServletMappingDecoded("/simple", "simple"); + Tomcat.addServlet(ctxt, "large", new LargeHeaderServlet()); + ctxt.addServletMappingDecoded("/large", "large"); + + tomcat.start(); + + openClientConnection(); + doHttpUpgrade(); + sendClientPreface(); + validateHttp2InitialResponse(); + + byte[] frameHeader = new byte[9]; + ByteBuffer headersPayload = ByteBuffer.allocate(128); + buildGetRequest(frameHeader, headersPayload, null, 3, "/large"); + writeFrame(frameHeader, headersPayload); + + // Headers + parser.readFrame(true); + parser.readFrame(true); + // Body + parser.readFrame(true); + + Assert.assertEquals( + "3-HeadersStart\n" + + "3-Header-[:status]-[200]\n" + + "3-Header-[x-ignore]-[...]\n" + + "3-Header-[content-type]-[text/plain;charset=UTF-8]\n" + + "3-Header-[date]-[Wed, 11 Nov 2015 19:18:42 GMT]\n" + + "3-HeadersEnd\n" + + "3-Body-2\n" + + "3-EndOfStream\n", output.getTrace()); + } + +} Propchange: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2UpgradeHandler.java ------------------------------------------------------------------------------ svn:eol-style = native Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1791124&r1=1791123&r2=1791124&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Wed Apr 12 14:25:50 2017 @@ -97,6 +97,10 @@ <fix> Align cipher configuration parsing with current OpenSSL master. (markt) </fix> + <fix> + <bug>60970</bug>: Fix infinite loop if application tries to write a + large header to the response when using HTTP/2. (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