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/01 11:24:59 UTC

svn commit: r1682847 - in /tomcat/trunk/java/org/apache/coyote/http2: ConnectionPrefaceParser.java Http2Protocol.java Http2UpgradeHandler.java LocalStrings.properties

Author: markt
Date: Mon Jun  1 09:24:58 2015
New Revision: 1682847

URL: http://svn.apache.org/r1682847
Log:
Remove the (overly complex) ConnectionPrefaceParser and use the new Http2Parser instead.
Initial unit tests identified a need to be able to set timeouts. Provide the ability to set these when configuring the UpgradeProtocol and take a first stab at some defaults (10s within an HTTP/2 frame, 30s between frames).

Removed:
    tomcat/trunk/java/org/apache/coyote/http2/ConnectionPrefaceParser.java
Modified:
    tomcat/trunk/java/org/apache/coyote/http2/Http2Protocol.java
    tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java
    tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties

Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2Protocol.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Http2Protocol.java?rev=1682847&r1=1682846&r2=1682847&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2Protocol.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2Protocol.java Mon Jun  1 09:24:58 2015
@@ -32,6 +32,11 @@ public class Http2Protocol implements Up
     private static final String ALPN_NAME = "h2";
     private static final byte[] ALPN_IDENTIFIER = ALPN_NAME.getBytes(StandardCharsets.UTF_8);
 
+    // All timeouts in milliseconds
+    private long readTimeout = 10000;
+    private long keepAliveTimeout = 30000;
+    private long writeTimeout = 10000;
+
     @Override
     public String getHttpUpgradeName(boolean isSecure) {
         if (isSecure) {
@@ -62,6 +67,42 @@ public class Http2Protocol implements Up
     @Override
     public InternalHttpUpgradeHandler getInteralUpgradeHandler(Adapter adapter,
             Request coyoteRequest) {
-        return new Http2UpgradeHandler(adapter, coyoteRequest);
+        Http2UpgradeHandler result = new Http2UpgradeHandler(adapter, coyoteRequest);
+
+        result.setReadTimeout(getReadTimeout());
+        result.setKeepAliveTimeout(getKeepAliveTimeout());
+        result.setWriteTimeout(getWriteTimeout());
+
+        return result;
+    }
+
+
+    public long getReadTimeout() {
+        return readTimeout;
+    }
+
+
+    public void setReadTimeout(long readTimeout) {
+        this.readTimeout = readTimeout;
+    }
+
+
+    public long getKeepAliveTimeout() {
+        return keepAliveTimeout;
+    }
+
+
+    public void setKeepAliveTimeout(long keepAliveTimeout) {
+        this.keepAliveTimeout = keepAliveTimeout;
+    }
+
+
+    public long getWriteTimeout() {
+        return writeTimeout;
+    }
+
+
+    public void setWriteTimeout(long writeTimeout) {
+        this.writeTimeout = writeTimeout;
     }
 }

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=1682847&r1=1682846&r2=1682847&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java Mon Jun  1 09:24:58 2015
@@ -32,10 +32,12 @@ import java.util.concurrent.atomic.Atomi
 import javax.servlet.http.WebConnection;
 
 import org.apache.coyote.Adapter;
+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.HpackEncoder.State;
+import org.apache.coyote.http2.Http2Parser.Input;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
 import org.apache.tomcat.util.codec.binary.Base64;
@@ -68,7 +70,8 @@ import org.apache.tomcat.util.res.String
  *
  * TODO: Review cookie parsing
  */
-public class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeHandler {
+public class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeHandler,
+        Input {
 
     private static final Log log = LogFactory.getLog(Http2UpgradeHandler.class);
     private static final StringManager sm = StringManager.getManager(Http2UpgradeHandler.class);
@@ -104,9 +107,9 @@ public class Http2UpgradeHandler extends
     private volatile SocketWrapperBase<?> socketWrapper;
     private volatile SSLSupport sslSupport;
 
+    private volatile Http2Parser parser;
+
     private volatile boolean initialized = false;
-    private volatile ConnectionPrefaceParser connectionPrefaceParser =
-            new ConnectionPrefaceParser();
     private volatile boolean firstFrame = true;
 
     private final ConnectionSettings remoteSettings = new ConnectionSettings();
@@ -117,6 +120,11 @@ public class Http2UpgradeHandler extends
     private ByteBuffer headerReadBuffer = ByteBuffer.allocate(1024);
     private HpackEncoder hpackEncoder;
 
+    // All timeouts in milliseconds
+    private long readTimeout = 10000;
+    private long keepAliveTimeout = 30000;
+    private long writeTimeout = 10000;
+
     private final Map<Integer,Stream> streams = new HashMap<>();
 
     // Tracking for when the connection is blocked (windowSize < 1)
@@ -146,9 +154,14 @@ public class Http2UpgradeHandler extends
             log.debug(sm.getString("upgradeHandler.init", Long.toString(connectionId)));
         }
 
+        parser = new Http2Parser(this);
+
         initialized = true;
         Stream stream = null;
 
+        socketWrapper.setReadTimeout(getReadTimeout());
+        socketWrapper.setWriteTimeout(getWriteTimeout());
+
         if (webConnection != null) {
             // HTTP/2 started via HTTP upgrade.
             // The initial HTTP/1.1 request is available as Stream 1.
@@ -166,7 +179,7 @@ public class Http2UpgradeHandler extends
                 if (settings.length % 6 != 0) {
                     // Invalid payload length for settings
                     // TODO i18n
-                    throw new IllegalStateException();
+                    throw new ProtocolException();
                 }
 
                 for (int i = 0; i < settings.length % 6; i++) {
@@ -179,13 +192,12 @@ public class Http2UpgradeHandler extends
                 hpackDecoder = new HpackDecoder(remoteSettings.getHeaderTableSize());
                 hpackEncoder = new HpackEncoder(localSettings.getHeaderTableSize());
 
-                if (!connectionPrefaceParser.parse(socketWrapper, true)) {
-                    // TODO i18n
-                    throw new IllegalStateException();
+                if (!parser.readConnectionPreface()) {
+                    throw new ProtocolException(sm.getString("upgradeHandler.invalidPreface"));
                 }
             } catch (IOException ioe) {
                 // TODO i18n
-                throw new IllegalStateException();
+                throw new ProtocolException();
             }
         }
 
@@ -235,22 +247,11 @@ public class Http2UpgradeHandler extends
 
         switch(status) {
         case OPEN_READ:
-            // Gets set to null once the connection preface has been
-            // successfully parsed.
-            if (connectionPrefaceParser != null) {
-                if (!connectionPrefaceParser.parse(socketWrapper, false)) {
-                    if (connectionPrefaceParser.isError()) {
-                        // Any errors will have already been logged.
-                        close();
-                        break;
-                    } else {
-                        // Incomplete
-                        result = SocketState.UPGRADED;
-                        break;
-                    }
-                }
+            if (!parser.readConnectionPreface()) {
+                close();
+                result = SocketState.CLOSED;
+                break;
             }
-            connectionPrefaceParser = null;
 
             // Process all the incoming data
             try {
@@ -336,6 +337,8 @@ public class Http2UpgradeHandler extends
         // TODO: Consider refactoring and making this a field to reduce GC.
         byte[] frameHeader = new byte[9];
         if (!getFrameHeader(frameHeader)) {
+            // Switch to keep-alive timeout between frames
+            socketWrapper.setReadTimeout(getKeepAliveTimeout());
             return false;
         }
 
@@ -694,6 +697,9 @@ public class Http2UpgradeHandler extends
             return false;
         }
 
+        // Switch to read timeout
+        socketWrapper.setReadTimeout(getReadTimeout());
+
         // Partial header read. Blocking within a frame to block while the
         // remainder is read.
         while (headerBytesRead < frameHeader.length) {
@@ -1039,4 +1045,67 @@ public class Http2UpgradeHandler extends
     protected final int getWeight() {
         return 0;
     }
+
+
+    // ------------------------------------------- Configuration getters/setters
+
+    public long getReadTimeout() {
+        return readTimeout;
+    }
+
+
+    public void setReadTimeout(long readTimeout) {
+        this.readTimeout = readTimeout;
+    }
+
+
+    public long getKeepAliveTimeout() {
+        return keepAliveTimeout;
+    }
+
+
+    public void setKeepAliveTimeout(long keepAliveTimeout) {
+        this.keepAliveTimeout = keepAliveTimeout;
+    }
+
+
+    public long getWriteTimeout() {
+        return writeTimeout;
+    }
+
+
+    public void setWriteTimeout(long writeTimeout) {
+        this.writeTimeout = writeTimeout;
+    }
+
+
+    // ----------------------------------------------- Http2Parser.Input methods
+
+    @Override
+    public boolean fill(byte[] data, boolean block) throws IOException {
+        int len = data.length;
+        int pos = 0;
+        boolean nextReadBlock = block;
+        int thisRead = 0;
+
+        while (len > 0) {
+            thisRead = socketWrapper.read(nextReadBlock, data, pos, len);
+            if (thisRead == 0) {
+                if (nextReadBlock) {
+                    // Should never happen
+                    throw new IllegalStateException();
+                } else {
+                    return false;
+                }
+            } else if (thisRead == -1) {
+                throw new EOFException();
+            } else {
+                pos += thisRead;
+                len -= thisRead;
+                nextReadBlock = true;
+            }
+        }
+
+        return 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=1682847&r1=1682846&r2=1682847&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties Mon Jun  1 09:24:58 2015
@@ -31,6 +31,9 @@ hpackdecoder.zeroNotValidHeaderTableInde
 
 hpackhuffman.huffmanEncodedHpackValueDidNotEndWithEOS=Huffman encoded value in HPACK headers did not end with EOS padding
 
+http2Parser.preface.invalid=Invalid connection preface [{0}] presented
+http2Parser.preface.io=Unable to read connection preface
+
 stream.header.debug=Connection [{0}], Stream [{1}], HTTP header [{2}], Value [{3}]
 stream.write=Connection [{0}], Stream [{1}]
 
@@ -39,6 +42,7 @@ 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.payloadTooBig=The payload is [{0}] bytes long but the maximum frame size is [{1}]
 upgradeHandler.processFrame=Connection [{0}], Stream [{1}], Flags [{2}], Payload size [{3}]
 upgradeHandler.processFrameData.invalidStream=Data frame received for stream [0]



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