Author: markt Date: Thu May 4 09:22:45 2017 New Revision: 1793758 URL: http://svn.apache.org/viewvc?rev=1793758&view=rev Log: Refactor to reduce duplication prior to adding trailer header support.
Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java tomcat/trunk/java/org/apache/coyote/http2/Stream.java 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=1793758&r1=1793757&r2=1793758&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java (original) +++ tomcat/trunk/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java Thu May 4 09:22:45 2017 @@ -25,8 +25,8 @@ import java.util.concurrent.TimeUnit; import org.apache.coyote.Adapter; import org.apache.coyote.ProtocolException; import org.apache.coyote.Request; -import org.apache.coyote.Response; import org.apache.coyote.http2.HpackEncoder.State; +import org.apache.tomcat.util.http.MimeHeaders; import org.apache.tomcat.util.net.SocketWrapperBase; import org.apache.tomcat.util.net.SocketWrapperBase.BlockingMode; @@ -133,88 +133,49 @@ public class Http2AsyncUpgradeHandler ex @Override - void writeHeaders(Stream stream, Response coyoteResponse, boolean endOfStream, int payloadSize) - throws IOException { + void writeHeaders(Stream stream, int pushedStreamId, MimeHeaders mimeHeaders, + boolean endOfStream, int payloadSize) throws IOException { + if (log.isDebugEnabled()) { log.debug(sm.getString("upgradeHandler.writeHeaders", connectionId, - stream.getIdentifier())); + stream.getIdentifier(), Integer.valueOf(pushedStreamId), + Boolean.valueOf(endOfStream))); } if (!stream.canWrite()) { return; } - boolean first = true; - State state = null; - ArrayList<ByteBuffer> bufs = new ArrayList<>(); - // 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); - state = getHpackEncoder().encode(coyoteResponse.getMimeHeaders(), target); - target.flip(); - if (state == State.COMPLETE || target.limit() > 0) { - ByteUtil.setThreeBytes(header, 0, target.limit()); - if (first) { - first = false; - header[3] = FrameType.HEADERS.getIdByte(); - if (endOfStream) { - 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()); - bufs.add(ByteBuffer.wrap(header)); - bufs.add(target); - } else if (state == State.UNDERFLOW) { - payloadSize = payloadSize * 2; - } - } - socketWrapper.write(BlockingMode.SEMI_BLOCK, getWriteTimeout(), TimeUnit.MILLISECONDS, - null, SocketWrapperBase.COMPLETE_WRITE, applicationErrorCompletion, - bufs.toArray(BYTEBUFFER_ARRAY)); - handleAsyncException(); - } - - - @Override - protected void writePushHeaders(Stream stream, int pushedStreamId, Request coyoteRequest, int payloadSize) - throws IOException { - if (log.isDebugEnabled()) { - log.debug(sm.getString("upgradeHandler.writePushHeaders", connectionId, - stream.getIdentifier(), Integer.toString(pushedStreamId))); - } - - if (!stream.canWrite()) { - return; + byte[] pushedStreamIdBytes = null; + if (pushedStreamId > 0) { + pushedStreamIdBytes = new byte[4]; + ByteUtil.set31Bits(pushedStreamIdBytes, 0, pushedStreamId); } 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); - if (first) { - target.put(pushedStreamIdBytes); + ByteBuffer payload = ByteBuffer.allocate(payloadSize); + if (first && pushedStreamIdBytes != null) { + payload.put(pushedStreamIdBytes); } - state = getHpackEncoder().encode(coyoteRequest.getMimeHeaders(), target); - target.flip(); - if (state == State.COMPLETE || target.limit() > 0) { - ByteUtil.setThreeBytes(header, 0, target.limit()); + state = getHpackEncoder().encode(mimeHeaders, payload); + payload.flip(); + if (state == State.COMPLETE || payload.limit() > 0) { + ByteUtil.setThreeBytes(header, 0, payload.limit()); if (first) { first = false; - header[3] = FrameType.PUSH_PROMISE.getIdByte(); + if (pushedStreamIdBytes == null) { + header[3] = FrameType.HEADERS.getIdByte(); + } else { + header[3] = FrameType.PUSH_PROMISE.getIdByte(); + } + if (endOfStream) { + header[4] = FLAG_END_OF_STREAM; + } } else { header[3] = FrameType.CONTINUATION.getIdByte(); } @@ -222,11 +183,11 @@ public class Http2AsyncUpgradeHandler ex header[4] += FLAG_END_OF_HEADERS; } if (log.isDebugEnabled()) { - log.debug(target.limit() + " bytes"); + log.debug(payload.limit() + " bytes"); } ByteUtil.set31Bits(header, 5, stream.getIdentifier().intValue()); bufs.add(ByteBuffer.wrap(header)); - bufs.add(target); + bufs.add(payload); } else if (state == State.UNDERFLOW) { payloadSize = payloadSize * 2; } 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=1793758&r1=1793757&r2=1793758&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java (original) +++ tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java Thu May 4 09:22:45 2017 @@ -40,7 +40,6 @@ import org.apache.coyote.Adapter; import org.apache.coyote.CloseNowException; import org.apache.coyote.ProtocolException; import org.apache.coyote.Request; -import org.apache.coyote.Response; import org.apache.coyote.http11.upgrade.InternalHttpUpgradeHandler; import org.apache.coyote.http2.HpackDecoder.HeaderEmitter; import org.apache.coyote.http2.HpackEncoder.State; @@ -49,6 +48,7 @@ import org.apache.coyote.http2.Http2Pars import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.apache.tomcat.util.codec.binary.Base64; +import org.apache.tomcat.util.http.MimeHeaders; import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState; import org.apache.tomcat.util.net.SSLSupport; import org.apache.tomcat.util.net.SocketEvent; @@ -522,11 +522,13 @@ class Http2UpgradeHandler extends Abstra } } - void writeHeaders(Stream stream, Response coyoteResponse, boolean endOfStream, int payloadSize) - throws IOException { + void writeHeaders(Stream stream, int pushedStreamId, MimeHeaders mimeHeaders, + boolean endOfStream, int payloadSize) throws IOException { + if (log.isDebugEnabled()) { log.debug(sm.getString("upgradeHandler.writeHeaders", connectionId, - stream.getIdentifier())); + stream.getIdentifier(), Integer.valueOf(pushedStreamId), + Boolean.valueOf(endOfStream))); } if (!stream.canWrite()) { @@ -534,78 +536,37 @@ class Http2UpgradeHandler extends Abstra } byte[] header = new byte[9]; - ByteBuffer target = ByteBuffer.allocate(payloadSize); - boolean first = true; - State state = null; - // This ensures the Stream processing thread has control of the socket. - synchronized (socketWrapper) { - while (state != State.COMPLETE) { - state = getHpackEncoder().encode(coyoteResponse.getMimeHeaders(), target); - target.flip(); - if (state == State.COMPLETE || target.limit() > 0) { - ByteUtil.setThreeBytes(header, 0, target.limit()); - if (first) { - first = false; - header[3] = FrameType.HEADERS.getIdByte(); - if (endOfStream) { - 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); - } - } - if (state == State.UNDERFLOW && target.limit() == 0) { - target = ByteBuffer.allocate(target.capacity() * 2); - } else { - target.clear(); - } - } - } - } - + ByteBuffer payload = ByteBuffer.allocate(payloadSize); - protected void writePushHeaders(Stream stream, int pushedStreamId, Request coyoteRequest, int payloadSize) - throws IOException { - if (log.isDebugEnabled()) { - log.debug(sm.getString("upgradeHandler.writePushHeaders", connectionId, - stream.getIdentifier(), Integer.toString(pushedStreamId))); + byte[] pushedStreamIdBytes = null; + if (pushedStreamId > 0) { + pushedStreamIdBytes = new byte[4]; + ByteUtil.set31Bits(pushedStreamIdBytes, 0, pushedStreamId); } - if (!stream.canWrite()) { - return; - } - - 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) { - target.put(pushedStreamIdBytes); while (state != State.COMPLETE) { - state = getHpackEncoder().encode(coyoteRequest.getMimeHeaders(), target); - target.flip(); - if (state == State.COMPLETE || target.limit() > 0) { - ByteUtil.setThreeBytes(header, 0, target.limit()); + if (first && pushedStreamIdBytes != null) { + payload.put(pushedStreamIdBytes); + } + state = getHpackEncoder().encode(mimeHeaders, payload); + payload.flip(); + if (state == State.COMPLETE || payload.limit() > 0) { + ByteUtil.setThreeBytes(header, 0, payload.limit()); if (first) { first = false; - header[3] = FrameType.PUSH_PROMISE.getIdByte(); + if (pushedStreamIdBytes == null) { + header[3] = FrameType.HEADERS.getIdByte(); + } else { + header[3] = FrameType.PUSH_PROMISE.getIdByte(); + } + if (endOfStream) { + header[4] = FLAG_END_OF_STREAM; + } } else { header[3] = FrameType.CONTINUATION.getIdByte(); } @@ -613,21 +574,19 @@ class Http2UpgradeHandler extends Abstra header[4] += FLAG_END_OF_HEADERS; } if (log.isDebugEnabled()) { - log.debug(target.limit() + " bytes"); + log.debug(payload.limit() + " bytes"); } ByteUtil.set31Bits(header, 5, stream.getIdentifier().intValue()); try { socketWrapper.write(true, header, 0, header.length); - socketWrapper.write(true, target); + socketWrapper.write(true, payload); socketWrapper.flush(true); } catch (IOException ioe) { handleAppInitiatedIOException(ioe); } - } - if (state == State.UNDERFLOW && target.limit() == 0) { - target = ByteBuffer.allocate(target.capacity() * 2); - } else { - target.clear(); + payload.clear(); + } else if (state == State.UNDERFLOW) { + payload = ByteBuffer.allocate(payload.capacity() * 2); } } } @@ -1133,7 +1092,8 @@ class Http2UpgradeHandler extends Abstra Stream pushStream = createLocalStream(request); // TODO: Is 1k the optimal value? - writePushHeaders(associatedStream, pushStream.getIdentifier().intValue(), request, 1024); + writeHeaders(associatedStream, pushStream.getIdentifier().intValue(), + request.getMimeHeaders(), false, 1024); pushStream.sentPushPromise(); Modified: tomcat/trunk/java/org/apache/coyote/http2/Stream.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Stream.java?rev=1793758&r1=1793757&r2=1793758&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/Stream.java (original) +++ tomcat/trunk/java/org/apache/coyote/http2/Stream.java Thu May 4 09:22:45 2017 @@ -49,11 +49,13 @@ class Stream extends AbstractStream impl private static final int HEADER_STATE_REGULAR = 2; private static final int HEADER_STATE_TRAILER = 3; - private static final Response ACK_RESPONSE = new Response(); + private static final MimeHeaders ACK_HEADERS; static { - ACK_RESPONSE.setStatus(100); - prepareHeaders(ACK_RESPONSE); + Response response = new Response(); + response.setStatus(100); + prepareHeaders(response); + ACK_HEADERS = response.getMimeHeaders(); } private volatile int weight = Constants.DEFAULT_WEIGHT; @@ -391,12 +393,12 @@ class Stream extends AbstractStream impl prepareHeaders(coyoteResponse); boolean endOfStream = getOutputBuffer().hasNoBody(); // TODO: Is 1k the optimal value? - handler.writeHeaders(this, coyoteResponse, endOfStream, 1024); + handler.writeHeaders(this, 0, coyoteResponse.getMimeHeaders(), endOfStream, 1024); } final void writeAck() throws IOException { // TODO: Is 64 too big? Just the status header with compression - handler.writeHeaders(this, ACK_RESPONSE, false, 64); + handler.writeHeaders(this, 0, ACK_HEADERS, false, 64); } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org