Author: markt Date: Wed May 3 17:30:06 2017 New Revision: 1793682 URL: http://svn.apache.org/viewvc?rev=1793682&view=rev Log: Extend the fix for large headers to push requests. Align the header writing implementations a little, with a view to refactoring
Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java?rev=1793682&r1=1793681&r2=1793682&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java (original) +++ tomcat/trunk/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java Wed May 3 17:30:06 2017 @@ -193,34 +193,39 @@ public class Http2AsyncUpgradeHandler ex log.debug(sm.getString("upgradeHandler.writePushHeaders", connectionId, stream.getIdentifier(), Integer.toString(pushedStreamId))); } - // This ensures the Stream processing thread has control of the socket. + boolean first = true; State state = null; ArrayList<ByteBuffer> bufs = new ArrayList<>(); byte[] pushedStreamIdBytes = new byte[4]; ByteUtil.set31Bits(pushedStreamIdBytes, 0, pushedStreamId); + // This ensures the Stream processing thread has control of the socket. while (state != State.COMPLETE) { byte[] header = new byte[9]; ByteBuffer target = ByteBuffer.allocate(payloadSize); target.put(pushedStreamIdBytes); state = getHpackEncoder().encode(coyoteRequest.getMimeHeaders(), target); target.flip(); - ByteUtil.setThreeBytes(header, 0, target.limit()); - if (first) { - first = false; - header[3] = FrameType.PUSH_PROMISE.getIdByte(); - } else { - header[3] = FrameType.CONTINUATION.getIdByte(); - } - if (state == State.COMPLETE) { - header[4] += FLAG_END_OF_HEADERS; - } - if (log.isDebugEnabled()) { - log.debug(target.limit() + " bytes"); + if (state == State.COMPLETE || target.limit() > 0) { + ByteUtil.setThreeBytes(header, 0, target.limit()); + if (first) { + first = false; + header[3] = FrameType.PUSH_PROMISE.getIdByte(); + } 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()); + bufs.add(ByteBuffer.wrap(header)); + bufs.add(target); + } else if (state == State.UNDERFLOW) { + payloadSize = payloadSize * 2; } - ByteUtil.set31Bits(header, 5, stream.getIdentifier().intValue()); - bufs.add(ByteBuffer.wrap(header)); - bufs.add(target); } socketWrapper.write(BlockingMode.SEMI_BLOCK, getWriteTimeout(), TimeUnit.MILLISECONDS, null, SocketWrapperBase.COMPLETE_WRITE, applicationErrorCompletion, 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=1793682&r1=1793681&r2=1793682&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java (original) +++ tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java Wed May 3 17:30:06 2017 @@ -614,35 +614,43 @@ class Http2UpgradeHandler extends Abstra log.debug(sm.getString("upgradeHandler.writePushHeaders", connectionId, stream.getIdentifier(), Integer.toString(pushedStreamId))); } + + byte[] header = new byte[9]; + ByteBuffer target = ByteBuffer.allocate(payloadSize); + boolean first = true; + State state = null; + byte[] pushedStreamIdBytes = new byte[4]; + ByteUtil.set31Bits(pushedStreamIdBytes, 0, pushedStreamId); // This ensures the Stream processing thread has control of the socket. synchronized (socketWrapper) { - byte[] header = new byte[9]; - ByteBuffer target = ByteBuffer.allocate(payloadSize); - boolean first = true; - State state = null; - byte[] pushedStreamIdBytes = new byte[4]; - ByteUtil.set31Bits(pushedStreamIdBytes, 0, pushedStreamId); target.put(pushedStreamIdBytes); while (state != State.COMPLETE) { state = getHpackEncoder().encode(coyoteRequest.getMimeHeaders(), target); target.flip(); - ByteUtil.setThreeBytes(header, 0, target.limit()); - if (first) { - first = false; - header[3] = FrameType.PUSH_PROMISE.getIdByte(); - } else { - header[3] = FrameType.CONTINUATION.getIdByte(); - } - if (state == State.COMPLETE) { - header[4] += FLAG_END_OF_HEADERS; + if (state == State.COMPLETE || target.limit() > 0) { + ByteUtil.setThreeBytes(header, 0, target.limit()); + if (first) { + first = false; + header[3] = FrameType.PUSH_PROMISE.getIdByte(); + } 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()); + socketWrapper.write(true, header, 0, header.length); + socketWrapper.write(true, target); + socketWrapper.flush(true); } - if (log.isDebugEnabled()) { - log.debug(target.limit() + " bytes"); + if (state == State.UNDERFLOW && target.limit() == 0) { + target = ByteBuffer.allocate(target.capacity() * 2); + } else { + target.clear(); } - ByteUtil.set31Bits(header, 5, stream.getIdentifier().intValue()); - socketWrapper.write(true, header, 0, header.length); - socketWrapper.write(true, target); - socketWrapper.flush(true); } } } Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1793682&r1=1793681&r2=1793682&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Wed May 3 17:30:06 2017 @@ -118,6 +118,10 @@ lack of support when <code>certificateVerificationDepth</code> has been explicitly set. (markt) </fix> + <fix> + <bug>60970</bug>: Extend the fix for large headers to push requests. + (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