Author: markt
Date: Fri Jun 26 09:17:57 2015
New Revision: 1687720

URL: http://svn.apache.org/r1687720
Log:
Add unit tests for HTTP/2 Ping frames
Expose the payload for ping ack frames as well as non-ack

Added:
    tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_7.java   (with 
props)
Modified:
    tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java
    tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java
    tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java
    tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_5_5.java
    tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_5.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=1687720&r1=1687719&r2=1687720&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java Fri Jun 26 
09:17:57 2015
@@ -313,14 +313,10 @@ class Http2Parser {
 
 
     private void readPingFrame(int flags) throws IOException {
-        if (Flags.isAck(flags)) {
-            output.pingAck();
-        } else {
-            // Read the payload
-            byte[] payload = new byte[8];
-            input.fill(true, payload);
-            output.pingReceive(payload);
-        }
+        // Read the payload
+        byte[] payload = new byte[8];
+        input.fill(true, payload);
+        output.pingReceive(payload, Flags.isAck(flags));
     }
 
 
@@ -600,8 +596,7 @@ class Http2Parser {
         void settingsEnd(boolean ack) throws IOException;
 
         // Ping frames
-        void pingReceive(byte[] payload) throws IOException;
-        void pingAck();
+        void pingReceive(byte[] payload, boolean ack) throws IOException;
 
         // Goaway
         void goaway(int lastStreamId, long errorCode, String debugData);

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=1687720&r1=1687719&r2=1687720&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java 
(original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java Fri Jun 
26 09:17:57 2015
@@ -903,22 +903,18 @@ public class Http2UpgradeHandler extends
 
 
     @Override
-    public void pingReceive(byte[] payload) throws IOException {
-        // Echo it back
-        synchronized (socketWrapper) {
-            socketWrapper.write(true, PING_ACK, 0, PING_ACK.length);
-            socketWrapper.write(true, payload, 0, payload.length);
-            socketWrapper.flush(true);
+    public void pingReceive(byte[] payload, boolean ack) throws IOException {
+        if (!ack) {
+            // Echo it back
+            synchronized (socketWrapper) {
+                socketWrapper.write(true, PING_ACK, 0, PING_ACK.length);
+                socketWrapper.write(true, payload, 0, payload.length);
+                socketWrapper.flush(true);
+            }
         }
     }
 
 
-    @Override
-    public void pingAck() {
-        // TODO Auto-generated method stub
-    }
-
-
     @Override
     public void goaway(int lastStreamId, long errorCode, String debugData) {
         if (log.isDebugEnabled()) {

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=1687720&r1=1687719&r2=1687720&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java (original)
+++ tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java Fri Jun 26 
09:17:57 2015
@@ -57,10 +57,6 @@ public abstract class Http2TestBase exte
         { 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00 };
     static final String EMPTY_HTTP2_SETTINGS_HEADER;
 
-    private static final byte[] PING_FRAME = new byte[] {
-        0x00, 0x00, 0x08, 0x06, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
-
     static {
         byte[] empty = new byte[0];
         EMPTY_HTTP2_SETTINGS_HEADER = "HTTP2-Settings: " + 
Base64.encodeBase64String(empty) + "\r\n";
@@ -488,7 +484,24 @@ public abstract class Http2TestBase exte
 
 
     void sendPing() throws IOException {
-        os.write(PING_FRAME);
+        sendPing(0, false, new byte[8]);
+    }
+
+
+    void sendPing(int streamId, boolean ack, byte[] payload) throws 
IOException {
+        byte[] pingHeader = new byte[9];
+        // length
+        ByteUtil.setThreeBytes(pingHeader, 0, payload.length);
+        // Type
+        pingHeader[3] = FrameType.PING.getIdByte();
+        // Flags
+        if (ack) {
+            ByteUtil.setOneBytes(pingHeader, 4, 0x01);
+        }
+        // Stream
+        ByteUtil.set31Bits(pingHeader, 5, streamId);
+        os.write(pingHeader);
+        os.write(payload);
         os.flush();
     }
 
@@ -685,8 +698,12 @@ public abstract class Http2TestBase exte
 
 
         @Override
-        public void pingReceive(byte[] payload) {
-            trace.append("0-Ping-[");
+        public void pingReceive(byte[] payload, boolean ack) {
+            trace.append("0-Ping-");
+            if (ack) {
+                trace.append("Ack-");
+            }
+            trace.append('[');
             boolean first = true;
             for (byte b : payload) {
                 if (first) {
@@ -700,12 +717,6 @@ public abstract class Http2TestBase exte
         }
 
 
-        @Override
-        public void pingAck() {
-            trace.append("0-Ping-Ack\n");
-        }
-
-
         @Override
         public void goaway(int lastStreamId, long errorCode, String debugData) 
{
             trace.append("0-Goaway-[" + lastStreamId + "]-[" + errorCode + 
"]-[" + debugData + "]");

Modified: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_5_5.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_5_5.java?rev=1687720&r1=1687719&r2=1687720&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_5_5.java 
(original)
+++ tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_5_5.java Fri Jun 
26 09:17:57 2015
@@ -73,7 +73,7 @@ public class TestHttp2Section_5_5 extend
 
         parser.readFrame(true);
 
-        Assert.assertEquals("0-Ping-Ack\n", output.getTrace());
+        Assert.assertEquals("0-Ping-Ack-[0,0,0,0,0,0,0,0]\n", 
output.getTrace());
     }
 
 

Modified: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_5.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_5.java?rev=1687720&r1=1687719&r2=1687720&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_5.java 
(original)
+++ tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_5.java Fri Jun 
26 09:17:57 2015
@@ -20,7 +20,7 @@ import org.junit.Assert;
 import org.junit.Test;
 
 /**
- * Unit tests for Section 6.4 of
+ * Unit tests for Section 6.5 of
  * <a href="https://tools.ietf.org/html/rfc7540";>RFC 7540</a>.
  * <br>
  * The order of tests in this class is aligned with the order of the

Added: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_7.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_7.java?rev=1687720&view=auto
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_7.java (added)
+++ tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_7.java Fri Jun 
26 09:17:57 2015
@@ -0,0 +1,92 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.apache.coyote.http2;
+
+import java.nio.charset.StandardCharsets;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * Unit tests for Section 6.7 of
+ * <a href="https://tools.ietf.org/html/rfc7540";>RFC 7540</a>.
+ * <br>
+ * The order of tests in this class is aligned with the order of the
+ * requirements in the RFC.
+ */
+public class TestHttp2Section_6_7 extends Http2TestBase {
+
+
+    @Test
+    public void testPingFrame() throws Exception {
+        // HTTP2 upgrade
+        http2Connect();
+
+        sendPing(0, false, "01234567".getBytes(StandardCharsets.ISO_8859_1));
+
+        // Ping ack
+        parser.readFrame(true);
+
+        Assert.assertEquals("0-Ping-Ack-[48,49,50,51,52,53,54,55]\n", 
output.getTrace());
+    }
+
+
+    @Test
+    public void testPingFrameUnexpectedAck() throws Exception {
+        // HTTP2 upgrade
+        http2Connect();
+
+        sendPing(0, true, "01234567".getBytes(StandardCharsets.ISO_8859_1));
+        sendPing(0, false, "76543210".getBytes(StandardCharsets.ISO_8859_1));
+
+        // Ping ack (only for second ping)
+        parser.readFrame(true);
+
+        Assert.assertEquals("0-Ping-Ack-[55,54,53,52,51,50,49,48]\n", 
output.getTrace());
+    }
+
+
+    @Test
+    public void testPingFrameNonZeroStream() throws Exception {
+        // HTTP2 upgrade
+        http2Connect();
+
+        sendPing(1, false, "76543210".getBytes(StandardCharsets.ISO_8859_1));
+
+        // Go away
+        parser.readFrame(true);
+
+        Assert.assertTrue(output.getTrace(), output.getTrace().startsWith(
+                "0-Goaway-[1]-[" + Http2Error.PROTOCOL_ERROR.getCode() + 
"]-["));
+    }
+
+
+    @Test
+    public void testPingFrameWrongPayloadSize() throws Exception {
+        // HTTP2 upgrade
+        http2Connect();
+
+        sendPing(0, false, "6543210".getBytes(StandardCharsets.ISO_8859_1));
+
+        // Go away
+        parser.readFrame(true);
+
+        Assert.assertTrue(output.getTrace(), output.getTrace().startsWith(
+                "0-Goaway-[1]-[" + Http2Error.FRAME_SIZE_ERROR.getCode() + 
"]-["));
+    }
+
+}

Propchange: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_7.java
------------------------------------------------------------------------------
    svn:eol-style = native



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

Reply via email to