Switch the Http2UpgradeHandler to using ConnectionSettingsLocal

Fix TODO for sending non-default values in initial settings frame including updating tests
 

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1696459 13f79535-47bb-0310-9956-ffa450edef68

java/org/apache/coyote/http2/ConnectionSettingsRemote.java

@@ -20,6 +20,10 @@
 import org.apache.juli.logging.LogFactory;
 import org.apache.tomcat.util.res.StringManager;
 
+/**
+ * Represents the remote connection settings: i.e. the settings the server must
+ * use when communicating with the client.
+ */
 public class ConnectionSettingsRemote {
 
     private final Log log = LogFactory.getLog(ConnectionSettingsRemote.class);

java/org/apache/coyote/http2/Http2UpgradeHandler.java

@@ -115,7 +115,7 @@ public class Http2UpgradeHandler extends AbstractStream implements InternalHttpU
     private volatile long pausedNanoTime = Long.MAX_VALUE;
 
     private final ConnectionSettingsRemote remoteSettings = new ConnectionSettingsRemote();
-    private final ConnectionSettingsRemote localSettings = new ConnectionSettingsRemote();
+    private final ConnectionSettingsLocal localSettings = new ConnectionSettingsLocal();
 
     private HpackDecoder hpackDecoder;
     private HpackEncoder hpackEncoder;
@@ -205,9 +205,9 @@ public void init(WebConnection webConnection) {
         }
 
         // Send the initial settings frame
-        // TODO: Need to send non-default settings values
         try {
-            socketWrapper.write(true, SETTINGS_EMPTY, 0, SETTINGS_EMPTY.length);
+            byte[] settings = localSettings.getSettingsFrameForPending();
+            socketWrapper.write(true, settings, 0, settings.length);
             socketWrapper.flush(true);
         } catch (IOException ioe) {
             throw new IllegalStateException(sm.getString("upgradeHandler.sendPrefaceFail"), ioe);
@@ -810,25 +810,13 @@ public void setWriteTimeout(long writeTimeout) {
     }
 
 
-    /*
-     * This only has an effect if called before the connection is established
-     */
     public void setMaxConcurrentStreams(long maxConcurrentStreams) {
         localSettings.setMaxConcurrentStreams(maxConcurrentStreams);
     }
 
 
-    /*
-     * This only has an effect if called before the connection is established
-     */
     public void setInitialWindowSize(int initialWindowSize) {
-        try {
-            localSettings.setInitialWindowSize(initialWindowSize);
-        } catch (ConnectionException e) {
-            // Illegal setting. Ignore it but log a warning.
-            log.warn(sm.getString("upgradeHandler.initialWindowSize.invalid",
-                    connectionId, Integer.toString(initialWindowSize)));
-        }
+        localSettings.setInitialWindowSize(initialWindowSize);
     }
 
 
@@ -995,7 +983,7 @@ public void setting(int identifier, long value) throws ConnectionException {
     @Override
     public void settingsEnd(boolean ack) throws IOException {
         if (ack) {
-            // TODO Process ACK
+            localSettings.ack();
         } else {
             synchronized (socketWrapper) {
                 socketWrapper.write(true, SETTINGS_ACK, 0, SETTINGS_ACK.length);

test/org/apache/coyote/http2/Http2TestBase.java

@@ -92,9 +92,10 @@ protected void http2Connect() throws Exception {
 
     protected void validateHttp2InitialResponse() throws Exception {
         // - 101 response acts as acknowledgement of the HTTP2-Settings header
-        // Need to read 4 frames
+        // Need to read 5 frames
         // - settings (server settings - must be first)
         // - settings ack (for the settings frame in the client preface)
+        // - ping
         // - headers (for response)
         // - data (for response body)
         parser.readFrame(true);
@@ -103,7 +104,8 @@ protected void validateHttp2InitialResponse() throws Exception {
         parser.readFrame(true);
         parser.readFrame(true);
 
-        Assert.assertEquals("0-Settings-End\n" +
+        Assert.assertEquals("0-Settings-[3]-[200]\n" +
+                "0-Settings-End\n" +
                 "0-Settings-Ack\n" +
                 "0-Ping-[0,0,0,0,0,0,0,1]\n" +
                 getSimpleResponseTrace(1)
@@ -596,16 +598,16 @@ void sendPriority(int streamId, int streamDependencyId, int weight) throws IOExc
 
 
     void sendSettings(int streamId, boolean ack, Setting... settings) throws IOException {
-        byte[] settingFrame = new byte[15];
         // length
-
         int settingsCount;
         if (settings == null) {
             settingsCount = 0;
         } else {
             settingsCount = settings.length;
         }
 
+        byte[] settingFrame = new byte[9 + 6 * settingsCount];
+
         ByteUtil.setThreeBytes(settingFrame, 0, 6 * settingsCount);
         // type
         settingFrame[3] = FrameType.SETTINGS.getIdByte();
@@ -732,6 +734,12 @@ public void settingsEnd(boolean ack) {
                 trace.append("0-Settings-Ack\n");
             } else {
                 trace.append("0-Settings-End\n");
+                try {
+                    sendSettings(0,  true);
+                } catch (IOException ioe) {
+                    // Convert to uncaught exception
+                    throw new IllegalStateException(ioe);
+                }
             }
         }
 

test/org/apache/coyote/http2/TestHttp2Section_5_1.java

@@ -199,7 +199,21 @@ public void testExceedMaxActiveStreams() throws Exception {
         openClientConnection();
         doHttpUpgrade();
         sendClientPreface();
-        validateHttp2InitialResponse();
+
+        // validateHttp2InitialResponse() - modified
+        parser.readFrame(true);
+        parser.readFrame(true);
+        parser.readFrame(true);
+        parser.readFrame(true);
+        parser.readFrame(true);
+
+        Assert.assertEquals("0-Settings-[3]-[1]\n" +
+                "0-Settings-End\n" +
+                "0-Settings-Ack\n" +
+                "0-Ping-[0,0,0,0,0,0,0,1]\n" +
+                getSimpleResponseTrace(1)
+                , output.getTrace());
+        output.clearTrace();
 
         sendLargeGetRequest(3);