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/02 15:44:17 UTC

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

Author: markt
Date: Tue Jun  2 13:44:16 2015
New Revision: 1683114

URL: http://svn.apache.org/r1683114
Log:
More work on the setting up h2c connections
- clients must send a settings frame in the preface (as well as the upgrade)
- refactor readFrame() so an expected type can be specified
- ensure the first frame the server sends is a settings frame
- restore sending the ack for a settings frame

Modified:
    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_3_2.java
    tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2_1.java

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=1683114&r1=1683113&r2=1683114&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java Tue Jun  2 13:44:16 2015
@@ -72,6 +72,10 @@ class Http2Parser {
      * @throws IOException If an IO error occurs while trying to read a frame
      */
     boolean readFrame(boolean block) throws IOException {
+        return readFrame(block, null);
+    }
+
+    private boolean readFrame(boolean block, Integer expected) throws IOException {
         if (!input.fill(block, frameHeaderBuffer)) {
             return false;
         }
@@ -81,6 +85,12 @@ class Http2Parser {
         int flags = ByteUtil.getOneByte(frameHeaderBuffer, 4);
         int streamId = ByteUtil.get31Bits(frameHeaderBuffer, 5);
 
+        if (expected != null && frameType != expected.intValue()) {
+            throw new Http2Exception(sm.getString("http2Parser.processFrame.unexpectedType",
+                    expected, Integer.toString(frameType)),
+                    streamId, Http2Exception.PROTOCOL_ERROR);
+        }
+
         if (payloadSize > maxPayloadSize) {
             throw new Http2Exception(sm.getString("http2Parser.payloadTooBig",
                     Integer.toString(payloadSize), Integer.toString(maxPayloadSize)),
@@ -282,15 +292,13 @@ class Http2Parser {
             throw new Http2Exception(sm.getString("http2Parser.processFrameSettings.invalidPayloadSize",
                     Integer.toString(payloadSize)), 0, Http2Exception.FRAME_SIZE_ERROR);
         }
-        if (payloadSize > 0 && (flags & 0x1) != 0) {
+        boolean ack = (flags & 0x1) != 0;
+        if (payloadSize > 0 && ack) {
             throw new Http2Exception(sm.getString("http2Parser.processFrameSettings.ackWithNonZeroPayload"),
                     0, Http2Exception.FRAME_SIZE_ERROR);
         }
 
-        if (payloadSize == 0) {
-            // Either an ACK or an empty settings frame
-            output.settingsEmpty((flags & 0x1) != 0);
-        } else {
+        if (payloadSize != 0) {
             // Process the settings
             byte[] setting = new byte[6];
             for (int i = 0; i < payloadSize / 6; i++) {
@@ -300,6 +308,7 @@ class Http2Parser {
                 output.setting(id, value);
             }
         }
+        output.settingsEnd(ack);
     }
 
 
@@ -389,7 +398,7 @@ class Http2Parser {
      *
      * @return <code>true</code> if a valid preface was read, otherwise false.
      */
-    boolean readConnectionPreface() {
+    boolean readConnectionPreface() throws IOException {
         if (readPreface) {
             return true;
         }
@@ -414,6 +423,9 @@ class Http2Parser {
             }
         }
 
+        // Must always be followed by a settings frame
+        readFrame(true, Integer.valueOf(FRAME_TYPE_SETTINGS));
+
         readPreface = true;
         return true;
     }
@@ -480,8 +492,8 @@ class Http2Parser {
         void headersEnd(int streamId);
 
         // Settings frames
-        void settingsEmpty(boolean ack);
         void setting(int identifier, long value) throws IOException;
+        void settingsEnd(boolean ack) throws IOException;
 
         // Ping frames
         void pingReceive(byte[] payload) throws IOException;

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=1683114&r1=1683113&r2=1683114&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java Tue Jun  2 13:44:16 2015
@@ -91,6 +91,7 @@ public class Http2UpgradeHandler extends
     private static final byte[] PING_ACK = { 0x00, 0x00, 0x08, 0x06, 0x01, 0x00, 0x00, 0x00, 0x00 };
 
     private static final byte[] SETTINGS_EMPTY = { 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00 };
+    private static final byte[] SETTINGS_ACK = { 0x00, 0x00, 0x00, 0x04, 0x01, 0x00, 0x00, 0x00, 0x00 };
 
     private static final byte[] GOAWAY = { 0x07, 0x00, 0x00, 0x00, 0x00 };
 
@@ -182,10 +183,6 @@ public class Http2UpgradeHandler extends
                     long value = ByteUtil.getFourBytes(settings, (i * 6) + 2);
                     remoteSettings.set(id, value);
                 }
-
-                if (!parser.readConnectionPreface()) {
-                    throw new ProtocolException(sm.getString("upgradeHandler.invalidPreface"));
-                }
             } catch (IOException ioe) {
                 // TODO i18n
                 throw new ProtocolException();
@@ -196,7 +193,6 @@ public class Http2UpgradeHandler extends
         try {
             socketWrapper.write(true, SETTINGS_EMPTY, 0, SETTINGS_EMPTY.length);
             socketWrapper.flush(true);
-
         } catch (IOException ioe) {
             throw new IllegalStateException(sm.getString("upgradeHandler.sendPrefaceFail"), ioe);
         }
@@ -237,14 +233,13 @@ public class Http2UpgradeHandler extends
 
         switch(status) {
         case OPEN_READ:
-            if (!parser.readConnectionPreface()) {
-                close();
-                result = SocketState.CLOSED;
-                break;
-            }
-
-            // Process all the incoming data
             try {
+                if (!parser.readConnectionPreface()) {
+                    close();
+                    result = SocketState.CLOSED;
+                    break;
+                }
+
                 while (parser.readFrame(false)) {
                 }
             } catch (Http2Exception h2e) {
@@ -751,16 +746,21 @@ public class Http2UpgradeHandler extends
 
 
     @Override
-    public void settingsEmpty(boolean ack) {
-        if (ack) {
-            // TODO Process ACK
-        }
+    public void setting(int identifier, long value) throws IOException {
+        remoteSettings.set(identifier, value);
     }
 
 
     @Override
-    public void setting(int identifier, long value) throws IOException {
-        remoteSettings.set(identifier, value);
+    public void settingsEnd(boolean ack) throws IOException {
+        if (ack) {
+            // TODO Process ACK
+        } else {
+            synchronized (socketWrapper) {
+                socketWrapper.write(true, SETTINGS_ACK, 0, SETTINGS_ACK.length);
+                socketWrapper.flush(true);
+            }
+        }
     }
 
 

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=1683114&r1=1683113&r2=1683114&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties Tue Jun  2 13:44:16 2015
@@ -36,6 +36,7 @@ http2Parser.payloadTooBig=The payload is
 http2Parser.preface.invalid=Invalid connection preface [{0}] presented
 http2Parser.preface.io=Unable to read connection preface
 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.processFrameHeaders.invalidStream=Headers frame received for stream [0]
 http2Parser.processFrameHeaders.decodingFailed=There was an error during the HPACK decoding of HTTP headers
@@ -59,8 +60,6 @@ streamProcessor.httpupgrade.notsupported
 upgradeHandler.connectionError=An error occurred that requires the HTTP/2 connection to be closed.
 upgradeHandler.init=Connection [{0}]
 upgradeHandler.ioerror=Connection [{0}]
-upgradeHandler.invalidPreface=And invalid connection preface was received from the client
-upgradeHandler.receivePrefaceNotSettings=The first frame received from the client was not a settings frame
 upgradeHandler.sendPrefaceFail=Failed to send preface to client
 upgradeHandler.socketCloseFailed=Error closing socket
 upgradeHandler.unexpectedEos=Unexpected end of stream

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=1683114&r1=1683113&r2=1683114&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java (original)
+++ tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java Tue Jun  2 13:44:16 2015
@@ -49,11 +49,13 @@ import org.apache.tomcat.util.codec.bina
  */
 public abstract class Http2TestBase extends TomcatBaseTest {
 
-    static final String EMPTY_HTTP2_SETTINGS;
+    private static final byte[] EMPTY_SETTINGS_FRAME =
+        { 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00 };
+    static final String EMPTY_HTTP2_SETTINGS_HEADER;
 
     static {
         byte[] empty = new byte[0];
-        EMPTY_HTTP2_SETTINGS = "HTTP2-Settings: " + Base64.encodeBase64String(empty) + "\r\n";
+        EMPTY_HTTP2_SETTINGS_HEADER = "HTTP2-Settings: " + Base64.encodeBase64String(empty) + "\r\n";
     }
 
     private Socket s;
@@ -73,12 +75,19 @@ public abstract class Http2TestBase exte
         openClientConnection();
         doHttpUpgrade();
         sendClientPreface();
-        // Need to read 3 frames (settings, headers and response body)
+        // - 101 response acts as acknowledgement of the HTTP2-Settings header
+        // Need to read 4 frames
+        // - settings (server settings - must be first)
+        // - settings ack (for the settings frame in the client preface)
+        // - headers (for response)
+        // - data (for response body)
+        parser.readFrame(true);
         parser.readFrame(true);
         parser.readFrame(true);
         parser.readFrame(true);
 
-        Assert.assertEquals("0-Settings-Empty\n" +
+        Assert.assertEquals("0-Settings-End\n" +
+                "0-Settings-Ack\n" +
                 "1-HeadersStart\n" +
                 "1-Header-[:status]-[200]\n" +
                 "1-HeadersEnd\n" +
@@ -125,7 +134,7 @@ public abstract class Http2TestBase exte
 
 
     protected void doHttpUpgrade() throws IOException {
-        doHttpUpgrade("h2c", EMPTY_HTTP2_SETTINGS, true);
+        doHttpUpgrade("h2c", EMPTY_HTTP2_SETTINGS_HEADER, true);
     }
 
     protected void doHttpUpgrade(String upgrade, String settings, boolean validate)
@@ -234,6 +243,7 @@ public abstract class Http2TestBase exte
 
     private void sendClientPreface() throws IOException {
         os.write(Http2Parser.CLIENT_PREFACE_START);
+        os.write(EMPTY_SETTINGS_FRAME);
         os.flush();
     }
 
@@ -322,19 +332,19 @@ public abstract class Http2TestBase exte
 
 
         @Override
-        public void settingsEmpty(boolean ack) {
-            if (ack) {
-                trace.append("0-Settings-Ack\n");
-            } else {
-                trace.append("0-Settings-Empty\n");
-            }
+        public void setting(int identifier, long value) throws IOException {
+            trace.append("0-Settings-[" + identifier + "]-[" + value + "]\n");
+            remoteSettings.set(identifier, value);
         }
 
 
         @Override
-        public void setting(int identifier, long value) throws IOException {
-            trace.append("0-Settings-[" + identifier + "]-[" + value + "]\n");
-            remoteSettings.set(identifier, value);
+        public void settingsEnd(boolean ack) {
+            if (ack) {
+                trace.append("0-Settings-Ack\n");
+            } else {
+                trace.append("0-Settings-End\n");
+            }
         }
 
 

Modified: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2.java?rev=1683114&r1=1683113&r2=1683114&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2.java (original)
+++ tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2.java Tue Jun  2 13:44:16 2015
@@ -39,7 +39,7 @@ public class TestHttp2Section_3_2 extend
     public void testConnectionNoHttp2Support() throws Exception {
         configureAndStartWebApplication();
         openClientConnection();
-        doHttpUpgrade("h2c", EMPTY_HTTP2_SETTINGS, false);
+        doHttpUpgrade("h2c", EMPTY_HTTP2_SETTINGS_HEADER, false);
         parseHttp11Response();
     }
 
@@ -49,7 +49,7 @@ public class TestHttp2Section_3_2 extend
         enableHttp2();
         configureAndStartWebApplication();
         openClientConnection();
-        doHttpUpgrade("h2", EMPTY_HTTP2_SETTINGS, false);
+        doHttpUpgrade("h2", EMPTY_HTTP2_SETTINGS_HEADER, false);
         parseHttp11Response();
     }
 

Modified: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2_1.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2_1.java?rev=1683114&r1=1683113&r2=1683114&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2_1.java (original)
+++ tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2_1.java Tue Jun  2 13:44:16 2015
@@ -35,8 +35,8 @@ public class TestHttp2Section_3_2_1 exte
         enableHttp2();
         configureAndStartWebApplication();
         openClientConnection();
-        doHttpUpgrade("h2c", Http2TestBase.EMPTY_HTTP2_SETTINGS + Http2TestBase.EMPTY_HTTP2_SETTINGS,
-                false);
+        doHttpUpgrade("h2c", Http2TestBase.EMPTY_HTTP2_SETTINGS_HEADER +
+                Http2TestBase.EMPTY_HTTP2_SETTINGS_HEADER, false);
         parseHttp11Response();
     }
 



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