You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2015/06/03 20:45:53 UTC

svn commit: r1683412 - in /tomcat/trunk: java/org/apache/coyote/http2/ test/org/apache/coyote/http2/

Author: markt
Date: Wed Jun  3 18:45:52 2015
New Revision: 1683412

URL: http://svn.apache.org/r1683412
Log:
Swtich to new Enum for error code
Add parsing for the Goaway frame (some TODOs in this code)
Enable test for header decoding issues that now passes

Modified:
    tomcat/trunk/java/org/apache/coyote/http2/ConnectionSettings.java
    tomcat/trunk/java/org/apache/coyote/http2/Http2Exception.java
    tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java
    tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java
    tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties
    tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java
    tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_4_3.java

Modified: tomcat/trunk/java/org/apache/coyote/http2/ConnectionSettings.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/ConnectionSettings.java?rev=1683412&r1=1683411&r2=1683412&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/ConnectionSettings.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/ConnectionSettings.java Wed Jun  3 18:45:52 2015
@@ -84,7 +84,7 @@ public class ConnectionSettings {
         // Need to put a sensible limit on this. Start with 16k (default is 4k)
         if (headerTableSize > (16 * 1024)) {
             throw new Http2Exception(sm.getString("connectionSettings.headerTableSizeLimit",
-                    Long.toString(headerTableSize)), 0, Http2Exception.PROTOCOL_ERROR);
+                    Long.toString(headerTableSize)), 0, ErrorCode.PROTOCOL_ERROR);
         }
         this.headerTableSize = (int) headerTableSize;
     }
@@ -98,7 +98,7 @@ public class ConnectionSettings {
         // will never be negative
         if (enablePush > 1) {
             throw new Http2Exception(sm.getString("connectionSettings.enablePushInvalid",
-                    Long.toString(enablePush)), 0, Http2Exception.PROTOCOL_ERROR);
+                    Long.toString(enablePush)), 0, ErrorCode.PROTOCOL_ERROR);
         }
         this.enablePush = (enablePush  == 1);
     }
@@ -119,7 +119,7 @@ public class ConnectionSettings {
         if (initialWindowSize > MAX_WINDOW_SIZE) {
             throw new Http2Exception(sm.getString("connectionSettings.windowSizeTooBig",
                     Long.toString(initialWindowSize), Long.toString(MAX_WINDOW_SIZE)),
-                    0, Http2Exception.PROTOCOL_ERROR);
+                    0, ErrorCode.PROTOCOL_ERROR);
         }
         this.initialWindowSize = (int) initialWindowSize;
     }
@@ -132,7 +132,7 @@ public class ConnectionSettings {
         if (maxFrameSize < MIN_MAX_FRAME_SIZE || maxFrameSize > MAX_MAX_FRAME_SIZE) {
             throw new Http2Exception(sm.getString("connectionSettings.maxFrameSizeInvalid",
                     Long.toString(maxFrameSize), Integer.toString(MIN_MAX_FRAME_SIZE),
-                    Integer.toString(MAX_MAX_FRAME_SIZE)), 0, Http2Exception.PROTOCOL_ERROR);
+                    Integer.toString(MAX_MAX_FRAME_SIZE)), 0, ErrorCode.PROTOCOL_ERROR);
         }
         this.maxFrameSize = (int) maxFrameSize;
     }

Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2Exception.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Http2Exception.java?rev=1683412&r1=1683411&r2=1683412&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2Exception.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2Exception.java Wed Jun  3 18:45:52 2015
@@ -22,27 +22,11 @@ public class Http2Exception extends IOEx
 
     private static final long serialVersionUID = 1L;
 
-    public static final byte[] NO_ERROR = { 0x00, 0x00, 0x00, 0x00 };
-    public static final byte[] PROTOCOL_ERROR = { 0x00, 0x00, 0x00, 0x01 };
-    public static final byte[] INTERNAL_ERROR = { 0x00, 0x00, 0x00, 0x02 };
-    public static final byte[] FLOW_CONTROL_ERROR = { 0x00, 0x00, 0x00, 0x03 };
-    public static final byte[] SETTINGS_TIMEOUT = { 0x00, 0x00, 0x00, 0x04 };
-    public static final byte[] STREAM_CLOSED = { 0x00, 0x00, 0x00, 0x05 };
-    public static final byte[] FRAME_SIZE_ERROR = { 0x00, 0x00, 0x00, 0x06};
-    public static final byte[] REFUSED_STREAM = { 0x00, 0x00, 0x00, 0x07};
-    public static final byte[] CANCEL = { 0x00, 0x00, 0x00, 0x08};
-    public static final byte[] COMPRESSION_ERROR= { 0x00, 0x00, 0x00, 0x09};
-    public static final byte[] CONNECT_ERROR = { 0x00, 0x00, 0x00, 0x0a};
-    public static final byte[] ENHANCE_YOUR_CALM = { 0x00, 0x00, 0x00, 0x0b};
-    public static final byte[] INADEQUATE_SECURITY = { 0x00, 0x00, 0x00, 0x0c};
-    public static final byte[]  HTTP_1_1_REQUIRED = { 0x00, 0x00, 0x00, 0x0d};
-
-
     private final int streamId;
-    private final byte[] errorCode;
+    private final ErrorCode errorCode;
 
 
-    public Http2Exception(String msg, int streamId, byte[] errorCode) {
+    public Http2Exception(String msg, int streamId, ErrorCode errorCode) {
         super(msg);
         this.streamId = streamId;
         this.errorCode = errorCode;
@@ -54,7 +38,7 @@ public class Http2Exception extends IOEx
     }
 
 
-    public byte[] getErrorCode() {
+    public ErrorCode getErrorCode() {
         return errorCode;
     }
 }

Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java?rev=1683412&r1=1683411&r2=1683412&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java Wed Jun  3 18:45:52 2015
@@ -38,6 +38,7 @@ class Http2Parser {
     private static final int FRAME_TYPE_PRIORITY = 2;
     private static final int FRAME_TYPE_SETTINGS = 4;
     private static final int FRAME_TYPE_PING = 6;
+    private static final int FRAME_TYPE_GOAWAY = 7;
     private static final int FRAME_TYPE_WINDOW_UPDATE = 8;
 
     private final String connectionId;
@@ -88,13 +89,13 @@ class Http2Parser {
         if (expected != null && frameType != expected.intValue()) {
             throw new Http2Exception(sm.getString("http2Parser.processFrame.unexpectedType",
                     expected, Integer.toString(frameType)),
-                    streamId, Http2Exception.PROTOCOL_ERROR);
+                    streamId, ErrorCode.PROTOCOL_ERROR);
         }
 
         if (payloadSize > maxPayloadSize) {
             throw new Http2Exception(sm.getString("http2Parser.payloadTooBig",
                     Integer.toString(payloadSize), Integer.toString(maxPayloadSize)),
-                    streamId, Http2Exception.FRAME_SIZE_ERROR);
+                    streamId, ErrorCode.FRAME_SIZE_ERROR);
         }
 
         switch (frameType) {
@@ -113,6 +114,9 @@ class Http2Parser {
         case FRAME_TYPE_PING:
             readPingFrame(streamId, flags, payloadSize);
             break;
+        case FRAME_TYPE_GOAWAY:
+            readGoawayFrame(streamId, flags, payloadSize);
+            break;
         case FRAME_TYPE_WINDOW_UPDATE:
             readWindowUpdateFrame(streamId, flags, payloadSize);
             break;
@@ -135,7 +139,7 @@ class Http2Parser {
         // Validate the stream
         if (streamId == 0) {
             throw new Http2Exception(sm.getString("http2Parser.processFrameData.invalidStream"),
-                    0, Http2Exception.PROTOCOL_ERROR);
+                    0, ErrorCode.PROTOCOL_ERROR);
         }
 
         // Process the Stream
@@ -180,7 +184,7 @@ class Http2Parser {
         // Validate the stream
         if (streamId == 0) {
             throw new Http2Exception(sm.getString("http2Parser.processFrameHeaders.invalidStream"),
-                    0, Http2Exception.PROTOCOL_ERROR);
+                    0, ErrorCode.PROTOCOL_ERROR);
         }
 
         // TODO Handle end of headers flag
@@ -230,7 +234,7 @@ class Http2Parser {
             } catch (HpackException hpe) {
                 throw new Http2Exception(
                         sm.getString("http2Parser.processFrameHeaders.decodingFailed"),
-                        0, Http2Exception.PROTOCOL_ERROR);
+                        0, ErrorCode.PROTOCOL_ERROR);
             }
             // switches to write mode
             headerReadBuffer.compact();
@@ -240,7 +244,7 @@ class Http2Parser {
         if (headerReadBuffer.position() > 0) {
             throw new Http2Exception(
                     sm.getString("http2Parser.processFrameHeaders.decodingDataLeft"),
-                    0, Http2Exception.PROTOCOL_ERROR);
+                    0, ErrorCode.PROTOCOL_ERROR);
         }
 
         swallow(padLength);
@@ -258,11 +262,11 @@ class Http2Parser {
         // Validate the frame
         if (streamId == 0) {
             throw new Http2Exception(sm.getString("http2Parser.processFramePriority.invalidStream"),
-                    0, Http2Exception.PROTOCOL_ERROR);
+                    0, ErrorCode.PROTOCOL_ERROR);
         }
         if (payloadSize != 5) {
             throw new Http2Exception(sm.getString("http2Parser.processFramePriority.invalidPayloadSize",
-                    Integer.toString(payloadSize)), streamId, Http2Exception.FRAME_SIZE_ERROR);
+                    Integer.toString(payloadSize)), streamId, ErrorCode.FRAME_SIZE_ERROR);
         }
 
         byte[] payload = new byte[5];
@@ -286,16 +290,16 @@ class Http2Parser {
         // Validate the frame
         if (streamId != 0) {
             throw new Http2Exception(sm.getString("http2Parser.processFrameSettings.invalidStream",
-                    Integer.toString(streamId)), 0, Http2Exception.FRAME_SIZE_ERROR);
+                    Integer.toString(streamId)), 0, ErrorCode.FRAME_SIZE_ERROR);
         }
         if (payloadSize % 6 != 0) {
             throw new Http2Exception(sm.getString("http2Parser.processFrameSettings.invalidPayloadSize",
-                    Integer.toString(payloadSize)), 0, Http2Exception.FRAME_SIZE_ERROR);
+                    Integer.toString(payloadSize)), 0, ErrorCode.FRAME_SIZE_ERROR);
         }
         boolean ack = (flags & 0x1) != 0;
         if (payloadSize > 0 && ack) {
             throw new Http2Exception(sm.getString("http2Parser.processFrameSettings.ackWithNonZeroPayload"),
-                    0, Http2Exception.FRAME_SIZE_ERROR);
+                    0, ErrorCode.FRAME_SIZE_ERROR);
         }
 
         if (payloadSize != 0) {
@@ -322,11 +326,11 @@ class Http2Parser {
         // Validate the frame
         if (streamId != 0) {
             throw new Http2Exception(sm.getString("http2Parser.processFramePing.invalidStream",
-                    Integer.toString(streamId)), 0, Http2Exception.FRAME_SIZE_ERROR);
+                    Integer.toString(streamId)), 0, ErrorCode.FRAME_SIZE_ERROR);
         }
         if (payloadSize != 8) {
             throw new Http2Exception(sm.getString("http2Parser.processFramePing.invalidPayloadSize",
-                    Integer.toString(payloadSize)), 0, Http2Exception.FRAME_SIZE_ERROR);
+                    Integer.toString(payloadSize)), 0, ErrorCode.FRAME_SIZE_ERROR);
         }
         if ((flags & 0x1) == 0) {
             // Read the payload
@@ -339,6 +343,37 @@ class Http2Parser {
     }
 
 
+    private void readGoawayFrame(int streamId, int flags, int payloadSize)
+            throws IOException {
+        if (log.isDebugEnabled()) {
+            log.debug(sm.getString("http2Parser.processFrame", connectionId,
+                    Integer.toString(streamId), Integer.toString(flags),
+                    Integer.toString(payloadSize)));
+        }
+
+        // Validate the frame
+        if (streamId != 0) {
+            throw new Http2Exception(sm.getString("http2Parser.processFrameGoaway.invalidStream",
+                    Integer.toString(streamId)), 0, ErrorCode.PROTOCOL_ERROR);
+        }
+
+        if (payloadSize < 8) {
+            throw new Http2Exception(sm.getString("http2Parser.processFrameGoaway.payloadTooSmall",
+                    connectionId, Integer.toString(payloadSize)), 0, ErrorCode.FRAME_SIZE_ERROR);
+        }
+
+        byte[] payload = new byte[payloadSize];
+        input.fill(true, payload);
+
+        int lastStreamId = ByteUtil.get31Bits(payload, 0);
+        long errorCode = ByteUtil.getFourBytes(payload, 4);
+        String debugData = null;
+        if (payloadSize > 8) {
+            debugData = new String(payload, 8, payloadSize - 8, StandardCharsets.UTF_8);
+        }
+        output.goaway(lastStreamId, errorCode, debugData);
+    }
+
     private void readWindowUpdateFrame(int streamId, int flags, int payloadSize)
             throws IOException {
         if (log.isDebugEnabled()) {
@@ -350,7 +385,7 @@ class Http2Parser {
         if (payloadSize != 4) {
             // Use stream 0 since this is always a connection error
             throw new Http2Exception(sm.getString("http2Parser.processFrameWindowUpdate.invalidPayloadSize",
-                    Integer.toString(payloadSize)), 0, Http2Exception.FRAME_SIZE_ERROR);
+                    Integer.toString(payloadSize)), 0, ErrorCode.FRAME_SIZE_ERROR);
         }
 
         byte[] payload = new byte[4];
@@ -365,7 +400,7 @@ class Http2Parser {
         // Validate the data
         if (windowSizeIncrement == 0) {
             throw new Http2Exception("http2Parser.processFrameWindowUpdate.invalidIncrement",
-                    streamId, Http2Exception.PROTOCOL_ERROR);
+                    streamId, ErrorCode.PROTOCOL_ERROR);
         }
 
         output.incrementWindowSize(streamId, windowSizeIncrement);
@@ -499,6 +534,9 @@ class Http2Parser {
         void pingReceive(byte[] payload) throws IOException;
         void pingAck();
 
+        // Goaway
+        void goaway(int lastStreamId, long errorCode, String debugData);
+
         // Window size
         void incrementWindowSize(int streamId, int increment);
 

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=1683412&r1=1683411&r2=1683412&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java Wed Jun  3 18:45:52 2015
@@ -122,6 +122,7 @@ public class Http2UpgradeHandler extends
     private long writeTimeout = 10000;
 
     private final Map<Integer,Stream> streams = new HashMap<>();
+    private int maxStreamId = -1;
 
     // Tracking for when the connection is blocked (windowSize < 1)
     private final Object backLogLock = new Object();
@@ -346,15 +347,20 @@ public class Http2UpgradeHandler extends
 
     private void close(Http2Exception h2e) {
         // Write a GOAWAY frame.
-        byte[] payload = h2e.getMessage().getBytes(StandardCharsets.UTF_8);
+        byte[] fixedPayload = new byte[8];
+        // TODO needs to be correct value
+        ByteUtil.set31Bits(fixedPayload, 0, (2 << 31) - 1);
+        ByteUtil.setFourBytes(fixedPayload, 4, h2e.getErrorCode().getErrorCode());
+        byte[] debugMessage = h2e.getMessage().getBytes(StandardCharsets.UTF_8);
         byte[] payloadLength = new byte[3];
-        ByteUtil.setThreeBytes(payloadLength, 0, payload.length);
+        ByteUtil.setThreeBytes(payloadLength, 0, debugMessage.length + 8);
 
         try {
             synchronized (socketWrapper) {
                 socketWrapper.write(true, payloadLength, 0, payloadLength.length);
                 socketWrapper.write(true, GOAWAY, 0, GOAWAY.length);
-                socketWrapper.write(true, payload, 0,  payload.length);
+                socketWrapper.write(true, fixedPayload, 0, 8);
+                socketWrapper.write(true, debugMessage, 0, debugMessage.length);
                 socketWrapper.flush(true);
             }
         } catch (IOException ioe) {
@@ -783,6 +789,18 @@ public class Http2UpgradeHandler extends
     }
 
 
+    @Override
+    public void goaway(int lastStreamId, long errorCode, String debugData) {
+        if (log.isDebugEnabled()) {
+            log.debug(sm.getString("upgradeHandler.goaway.debug", connectionId,
+                    Integer.toString(lastStreamId), Long.toHexString(errorCode), debugData));
+        }
+
+        // TODO: Do more than just record this
+        maxStreamId = lastStreamId;
+    }
+
+
     @Override
     public void incrementWindowSize(int streamId, int increment) {
         AbstractStream stream = getStream(streamId);

Modified: tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties?rev=1683412&r1=1683411&r2=1683412&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties Wed Jun  3 18:45:52 2015
@@ -38,6 +38,8 @@ http2Parser.preface.io=Unable to read co
 http2Parser.processFrame=Connection [{0}], Stream [{1}], Flags [{2}], Payload size [{3}]
 http2Parser.processFrame.unexpectedType=Expected frame type [{0}] but received frame type [{1}]
 http2Parser.processFrameData.invalidStream=Data frame received for stream [0]
+http2Parser.processFrameGoaway.invalidStream=Goaway frame received for stream [{0}]
+http2Parser.processFrameGoaway.payloadTooSmall=Connection [{0}]: Goaway payload size was [{1}] which is less than the minimum 8
 http2Parser.processFrameHeaders.invalidStream=Headers frame received for stream [0]
 http2Parser.processFrameHeaders.decodingFailed=There was an error during the HPACK decoding of HTTP headers
 http2Parser.processFrameHeaders.decodingDataLeft=Data left over after HPACK decoding - it should have been consumed
@@ -58,6 +60,7 @@ stream.write=Connection [{0}], Stream [{
 streamProcessor.httpupgrade.notsupported=HTTP upgrade is not supported within HTTP/2 streams
 
 upgradeHandler.connectionError=An error occurred that requires the HTTP/2 connection to be closed.
+upgradeHandler.goaway.debug=Connection [{0}], Goaway, Last stream [{1}], Error code [{2}], Debug data [{3}]
 upgradeHandler.init=Connection [{0}]
 upgradeHandler.ioerror=Connection [{0}]
 upgradeHandler.sendPrefaceFail=Failed to send preface to client

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=1683412&r1=1683411&r2=1683412&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java (original)
+++ tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java Wed Jun  3 18:45:52 2015
@@ -447,6 +447,12 @@ public abstract class Http2TestBase exte
 
 
         @Override
+        public void goaway(int lastStreamId, long errorCode, String debugData) {
+            trace.append("0-Goaway-[" + lastStreamId + "]-[" + errorCode + "]-[" + debugData + "]");
+        }
+
+
+        @Override
         public void incrementWindowSize(int streamId, int increment) {
             trace.append(streamId + "-WindowSize-[" + increment + "]\n");
         }

Modified: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_4_3.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_4_3.java?rev=1683412&r1=1683411&r2=1683412&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_4_3.java (original)
+++ tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_4_3.java Wed Jun  3 18:45:52 2015
@@ -19,7 +19,6 @@ package org.apache.coyote.http2;
 import java.nio.ByteBuffer;
 
 import org.junit.Assert;
-import org.junit.Ignore;
 import org.junit.Test;
 
 /**
@@ -31,7 +30,6 @@ import org.junit.Test;
  */
 public class TestHttp2Section_4_3 extends Http2TestBase {
 
-    @Ignore // Appears to trigger various problems
     @Test
     public void testHeaderDecodingError() throws Exception {
         hpackEncoder = new HpackEncoder(ConnectionSettings.DEFAULT_HEADER_TABLE_SIZE);
@@ -50,9 +48,11 @@ public class TestHttp2Section_4_3 extend
         // Process the request
         writeSimpleRequest(frameHeader, headersPayload);
 
-        readSimpleResponse();
-        Assert.assertEquals(getSimpleResponseTrace(3), output.getTrace());
+        // Read GOAWAY frame
+        parser.readFrame(true);
 
+        Assert.assertTrue(output.getTrace(),
+                output.getTrace().startsWith("0-Goaway-[2147483647]-[1]-["));
     }
 
 



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org