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/07/01 15:53:52 UTC

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

Author: markt
Date: Wed Jul  1 13:53:52 2015
New Revision: 1688655

URL: http://svn.apache.org/r1688655
Log:
Add some simple unit tests for goaway frames.
These required several changes to HTTP/2 processing
- tracking pings to match ACKs with original pings
- tracking round trip time
- regular (every 10s) pings to track changes to round trip time
- differentiate between pausing and paused states

Added:
    tomcat/trunk/java/org/apache/coyote/http2/HeaderSink.java   (with props)
    tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_8.java   (with props)
Modified:
    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

Added: tomcat/trunk/java/org/apache/coyote/http2/HeaderSink.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/HeaderSink.java?rev=1688655&view=auto
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/HeaderSink.java (added)
+++ tomcat/trunk/java/org/apache/coyote/http2/HeaderSink.java Wed Jul  1 13:53:52 2015
@@ -0,0 +1,32 @@
+/*
+ *  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 org.apache.coyote.http2.HpackDecoder.HeaderEmitter;
+
+/**
+ * Purpose of this class is to silently swallow any headers. It is used once
+ * the connection close process has started if headers for new streams are
+ * received.
+ */
+public class HeaderSink implements HeaderEmitter {
+
+    @Override
+    public void emitHeader(String name, String value, boolean neverIndex) {
+        // NO-OP
+    }
+}

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

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=1688655&r1=1688654&r2=1688655&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java Wed Jul  1 13:53:52 2015
@@ -25,9 +25,12 @@ import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Queue;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
 
 import javax.servlet.http.WebConnection;
 
@@ -84,6 +87,7 @@ public class Http2UpgradeHandler extends
     private static final int FLAG_END_OF_STREAM = 1;
     private static final int FLAG_END_OF_HEADERS = 4;
 
+    private static final byte[] PING = { 0x00, 0x00, 0x08, 0x06, 0x00, 0x00, 0x00, 0x00, 0x00};
     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 };
@@ -95,10 +99,7 @@ public class Http2UpgradeHandler extends
     private static final byte[] HTTP2_UPGRADE_ACK = ("HTTP/1.1 101 Switching Protocols\r\n" +
                 "Connection: Upgrade\r\nUpgrade: h2c\r\n\r\n").getBytes(StandardCharsets.ISO_8859_1);
 
-    private static final int STATE_NEW = 0;
-    private static final int STATE_CONNECTED = 1;
-    private static final int STATE_PAUSED = 2;
-    private static final int STATE_CLOSED =3;
+    private static final HeaderSink HEADER_SINK = new HeaderSink();
 
     private final String connectionId;
 
@@ -109,8 +110,9 @@ public class Http2UpgradeHandler extends
     private volatile Http2Parser parser;
 
     // Simple state machine (sequence of states)
-
-    private AtomicInteger connectionState = new AtomicInteger(STATE_NEW);
+    private AtomicReference<ConnectionState> connectionState =
+            new AtomicReference<>(ConnectionState.NEW);
+    private volatile long pausedNanoTime = Long.MAX_VALUE;
 
     private final ConnectionSettings remoteSettings = new ConnectionSettings();
     private final ConnectionSettings localSettings = new ConnectionSettings();
@@ -129,6 +131,7 @@ public class Http2UpgradeHandler extends
     // Start at -1 so the 'add 2' logic in closeIdleStreams() works
     private volatile int maxActiveRemoteStreamId = -1;
     private volatile int maxProcessedStreamId;
+    private final PingManager pingManager = new PingManager();
 
     // Tracking for when the connection is blocked (windowSize < 1)
     private final Map<AbstractStream,int[]> backLogStreams = new ConcurrentHashMap<>();
@@ -162,7 +165,7 @@ public class Http2UpgradeHandler extends
             log.debug(sm.getString("upgradeHandler.init", connectionId));
         }
 
-        if (!connectionState.compareAndSet(STATE_NEW, STATE_CONNECTED)) {
+        if (!connectionState.compareAndSet(ConnectionState.NEW, ConnectionState.CONNECTED)) {
             return;
         }
 
@@ -218,6 +221,13 @@ public class Http2UpgradeHandler extends
                     sm.getString("upgradeHandler.invalidPreface", connectionId));
         }
 
+        // Send a ping to get an idea of round trip time as early as possible
+        try {
+            pingManager.sendPing(true);
+        } catch (IOException ioe) {
+            throw new ProtocolException(sm.getString("upgradeHandler.pingFailed"), ioe);
+        }
+
         if (webConnection != null) {
             // Process the initial request on a container thread
             StreamProcessor streamProcessor = new StreamProcessor(stream, adapter, socketWrapper);
@@ -249,70 +259,69 @@ public class Http2UpgradeHandler extends
         // Might not be necessary. init() will handle that.
         init(null);
 
+
         SocketState result = SocketState.CLOSED;
 
-        switch(status) {
-        case OPEN_READ:
-            try {
-                while (true) {
-                    try {
-                        if (!parser.readFrame(false)) {
-                            break;
+        try {
+            pingManager.sendPing(false);
+
+            checkPauseState();
+
+            switch(status) {
+            case OPEN_READ:
+                try {
+
+                    while (true) {
+                        try {
+                            if (!parser.readFrame(false)) {
+                                break;
+                            }
+                        } catch (StreamException se) {
+                            // Stream errors are not fatal to the connection so
+                            // continue reading frames
+                            closeStream(se);
                         }
-                    } catch (StreamException se) {
-                        // Stream errors are not fatal to the connection so
-                        // continue reading frames
-                        closeStream(se);
                     }
+                } catch (Http2Exception ce) {
+                    // Really ConnectionError
+                    if (log.isDebugEnabled()) {
+                        log.debug(sm.getString("upgradeHandler.connectionError"), ce);
+                    }
+                    closeConnection(ce);
+                    break;
                 }
-            } catch (Http2Exception ce) {
-                // Really ConnectionError
-                if (log.isDebugEnabled()) {
-                    log.debug(sm.getString("upgradeHandler.connectionError"), ce);
-                }
-                closeConnection(ce);
-                break;
-            } catch (IOException ioe) {
-                if (log.isDebugEnabled()) {
-                    log.debug(sm.getString("upgradeHandler.ioerror", connectionId), ioe);
-                }
-                close();
-                break;
-            }
 
-            result = SocketState.UPGRADED;
-            break;
+                result = SocketState.UPGRADED;
+                break;
 
-        case OPEN_WRITE:
-            try {
+                case OPEN_WRITE:
                 processWrites();
-            } catch (IOException ioe) {
-                if (log.isDebugEnabled()) {
-                    log.debug(sm.getString("upgradeHandler.ioerror", connectionId), ioe);
-                }
+
+                result = SocketState.UPGRADED;
+                break;
+
+            case ASYNC_READ_ERROR:
+            case ASYNC_WRITE_ERROR:
+            case CLOSE_NOW:
+                // This should never happen and will be fatal for this connection.
+                // Add the exception to trace how this point was reached.
+                log.error(sm.getString("upgradeHandler.unexpectedStatus", status),
+                        new IllegalStateException());
+                //$FALL-THROUGH$
+            case DISCONNECT:
+            case ERROR:
+            case TIMEOUT:
+            case STOP:
+                // For all of the above, including the unexpected values, close the
+                // connection.
                 close();
                 break;
             }
-
-            result = SocketState.UPGRADED;
-            break;
-
-        case ASYNC_READ_ERROR:
-        case ASYNC_WRITE_ERROR:
-        case CLOSE_NOW:
-            // This should never happen and will be fatal for this connection.
-            // Add the exception to trace how this point was reached.
-            log.error(sm.getString("upgradeHandler.unexpectedStatus", status),
-                    new IllegalStateException());
-            //$FALL-THROUGH$
-        case DISCONNECT:
-        case ERROR:
-        case TIMEOUT:
-        case STOP:
-            // For all of the above, including the unexpected values, close the
-            // connection.
+        } catch (IOException ioe) {
+            if (log.isDebugEnabled()) {
+                log.debug(sm.getString("upgradeHandler.ioerror", connectionId), ioe);
+            }
             close();
-            break;
         }
 
         if (log.isDebugEnabled()) {
@@ -333,7 +342,29 @@ public class Http2UpgradeHandler extends
             log.debug(sm.getString("upgradeHandler.pause.entry", connectionId));
         }
 
-        connectionState.compareAndSet(STATE_CONNECTED, STATE_PAUSED);
+        if (connectionState.compareAndSet(ConnectionState.CONNECTED, ConnectionState.PAUSING)) {
+            pausedNanoTime = System.nanoTime();
+
+            // Write a GOAWAY frame.
+            byte[] fixedPayload = new byte[8];
+            ByteUtil.set31Bits(fixedPayload, 0, (1 << 31) - 1);
+            ByteUtil.setFourBytes(fixedPayload, 4, Http2Error.NO_ERROR.getCode());
+            byte[] payloadLength = new byte[3];
+            ByteUtil.setThreeBytes(payloadLength, 0, 8);
+
+            try {
+                synchronized (socketWrapper) {
+                    socketWrapper.write(true, payloadLength, 0, payloadLength.length);
+                    socketWrapper.write(true, GOAWAY, 0, GOAWAY.length);
+                    socketWrapper.write(true, fixedPayload, 0, 8);
+                    socketWrapper.flush(true);
+                }
+            } catch (IOException ioe) {
+                // This is fatal for the connection. Ignore it here. There will be
+                // further attempts at I/O in upgradeDispatch() and it can better
+                // handle the IO errors.
+            }
+        }
     }
 
 
@@ -343,6 +374,30 @@ public class Http2UpgradeHandler extends
     }
 
 
+    private void checkPauseState() throws IOException {
+        if (connectionState.get() == ConnectionState.PAUSING) {
+            if (pausedNanoTime + pingManager.getRoundTripTimeNano() < System.nanoTime()) {
+                connectionState.compareAndSet(ConnectionState.PAUSING, ConnectionState.PAUSED);
+
+                // Write a GOAWAY frame.
+                byte[] fixedPayload = new byte[8];
+                ByteUtil.set31Bits(fixedPayload, 0, maxProcessedStreamId);
+                ByteUtil.setFourBytes(fixedPayload, 4, Http2Error.NO_ERROR.getCode());
+                byte[] payloadLength = new byte[3];
+                ByteUtil.setThreeBytes(payloadLength, 0, 8);
+
+                synchronized (socketWrapper) {
+                    socketWrapper.write(true, payloadLength, 0, payloadLength.length);
+                    socketWrapper.write(true, GOAWAY, 0, GOAWAY.length);
+                    socketWrapper.write(true, fixedPayload, 0, 8);
+                    socketWrapper.flush(true);
+                }
+
+            }
+        }
+    }
+
+
     private void closeStream(StreamException se) throws ConnectionException, IOException {
 
         if (log.isDebugEnabled()) {
@@ -661,7 +716,7 @@ public class Http2UpgradeHandler extends
     }
 
 
-    private Stream getStream(int streamId, boolean unknownIsError) throws ConnectionException{
+    private Stream getStream(int streamId, boolean unknownIsError) throws ConnectionException {
         Integer key = Integer.valueOf(streamId);
         Stream result = streams.get(key);
         if (result == null && unknownIsError) {
@@ -696,7 +751,7 @@ public class Http2UpgradeHandler extends
 
 
     private void close() {
-        connectionState.set(STATE_CLOSED);
+        connectionState.set(ConnectionState.CLOSED);
         try {
             socketWrapper.close();
         } catch (IOException ioe) {
@@ -811,10 +866,12 @@ public class Http2UpgradeHandler extends
 
     @Override
     public void receiveEndOfStream(int streamId) throws ConnectionException {
-        Stream stream = getStream(streamId, true);
-        stream.receivedEndOfStream();
-        if (!stream.isActive()) {
-            activeRemoteStreamCount.decrementAndGet();
+        Stream stream = getStream(streamId, connectionState.get().isNewStreamAllowed());
+        if (stream != null) {
+            stream.receivedEndOfStream();
+            if (!stream.isActive()) {
+                activeRemoteStreamCount.decrementAndGet();
+            }
         }
     }
 
@@ -830,20 +887,29 @@ public class Http2UpgradeHandler extends
 
     @Override
     public HeaderEmitter headersStart(int streamId) throws Http2Exception {
-        Stream stream = getStream(streamId, false);
-        if (stream == null) {
-            stream = createRemoteStream(streamId);
-        }
-        stream.checkState(FrameType.HEADERS);
-        stream.receivedStartOfHeaders();
-        closeIdleStreams(streamId);
-        if (localSettings.getMaxConcurrentStreams() < activeRemoteStreamCount.incrementAndGet()) {
-            activeRemoteStreamCount.decrementAndGet();
-            throw new StreamException(sm.getString("upgradeHandler.tooManyRemoteStreams",
-                    Long.toString(localSettings.getMaxConcurrentStreams())),
-                    Http2Error.REFUSED_STREAM, streamId);
+        if (connectionState.get().isNewStreamAllowed()) {
+            Stream stream = getStream(streamId, false);
+            if (stream == null) {
+                stream = createRemoteStream(streamId);
+            }
+            stream.checkState(FrameType.HEADERS);
+            stream.receivedStartOfHeaders();
+            closeIdleStreams(streamId);
+            if (localSettings.getMaxConcurrentStreams() < activeRemoteStreamCount.incrementAndGet()) {
+                activeRemoteStreamCount.decrementAndGet();
+                throw new StreamException(sm.getString("upgradeHandler.tooManyRemoteStreams",
+                        Long.toString(localSettings.getMaxConcurrentStreams())),
+                        Http2Error.REFUSED_STREAM, streamId);
+            }
+            return stream;
+        } else {
+            if (log.isDebugEnabled()) {
+                log.debug(sm.getString("upgradeHandler.noNewStreams",
+                        connectionId, Integer.toString(streamId)));
+            }
+            // Stateless so a static can be used to save on GC
+            return HEADER_SINK;
         }
-        return stream;
     }
 
 
@@ -877,11 +943,13 @@ public class Http2UpgradeHandler extends
     @Override
     public void headersEnd(int streamId) throws ConnectionException {
         setMaxProcessedStream(streamId);
-        Stream stream = getStream(streamId, true);
-        // Process this stream on a container thread
-        StreamProcessor streamProcessor = new StreamProcessor(stream, adapter, socketWrapper);
-        streamProcessor.setSslSupport(sslSupport);
-        socketWrapper.getEndpoint().getExecutor().execute(streamProcessor);
+        Stream stream = getStream(streamId, connectionState.get().isNewStreamAllowed());
+        if (stream != null) {
+            // Process this stream on a container thread
+            StreamProcessor streamProcessor = new StreamProcessor(stream, adapter, socketWrapper);
+            streamProcessor.setSslSupport(sslSupport);
+            socketWrapper.getEndpoint().getExecutor().execute(streamProcessor);
+        }
     }
 
 
@@ -921,14 +989,7 @@ public class Http2UpgradeHandler extends
 
     @Override
     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);
-            }
-        }
+        pingManager.receivePing(payload, ack);
     }
 
 
@@ -958,4 +1019,115 @@ public class Http2UpgradeHandler extends
             throws IOException {
         // NO-OP.
     }
+
+
+    private class PingManager {
+
+        // 10 seconds
+        private final long pingIntervalNano = 10000000000L;
+
+        private int sequence = 0;
+        private long lastPingNanoTime = Long.MIN_VALUE;
+
+        private Queue<PingRecord> inflightPings = new ConcurrentLinkedQueue<>();
+        private Queue<Long> roundTripTimes = new ConcurrentLinkedQueue<>();
+
+        /**
+         * Check to see if a ping was sent recently and, if not, send one.
+         *
+         * @throws IOException If an I/O issue prevents the ping from being sent
+         */
+        public void sendPing(boolean force) throws IOException {
+            long now = System.nanoTime();
+            if (force || now - lastPingNanoTime > pingIntervalNano) {
+                lastPingNanoTime = now;
+                byte[] payload = new byte[8];
+                synchronized (socketWrapper) {
+                    int sentSequence = ++sequence;
+                    PingRecord pingRecord = new PingRecord(sentSequence, now);
+                    inflightPings.add(pingRecord);
+                    ByteUtil.set31Bits(payload, 4, sentSequence);
+                    socketWrapper.write(true, PING, 0, PING.length);
+                    socketWrapper.write(true, payload, 0, payload.length);
+                    socketWrapper.flush(true);
+                }
+            }
+        }
+
+        public void receivePing(byte[] payload, boolean ack) throws IOException {
+            if (ack) {
+                // Extract the sequence from the payload
+                int receivedSequence = ByteUtil.get31Bits(payload, 4);
+                PingRecord pingRecord = inflightPings.poll();
+                while (pingRecord != null && pingRecord.getSequence() < receivedSequence) {
+                    pingRecord = inflightPings.poll();
+                }
+                if (pingRecord == null) {
+                    // Unexpected ACK. Log it.
+                } else {
+                    long roundTripTime = System.nanoTime() - pingRecord.getSentNanoTime();
+                    roundTripTimes.add(Long.valueOf(roundTripTime));
+                    while (roundTripTimes.size() > 3) {
+                        roundTripTimes.poll();
+                    }
+                    if (log.isDebugEnabled()) {
+                        log.debug(sm.getString("pingManager.roundTripTime",
+                                connectionId, Long.valueOf(roundTripTime)));
+                    }
+                }
+
+            } else {
+                // Client originated ping. 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 long getRoundTripTimeNano() {
+            return (long) roundTripTimes.stream().mapToLong(x -> x.longValue()).average().orElse(0);
+        }
+    }
+
+
+    private static class PingRecord {
+
+        private final int sequence;
+        private final long sentNanoTime;
+
+        public PingRecord(int sequence, long sentNanoTime) {
+            this.sequence = sequence;
+            this.sentNanoTime = sentNanoTime;
+        }
+
+        public int getSequence() {
+            return sequence;
+        }
+
+        public long getSentNanoTime() {
+            return sentNanoTime;
+        }
+    }
+
+
+    private enum ConnectionState {
+
+        NEW(true),
+        CONNECTED(true),
+        PAUSING(true),
+        PAUSED(false),
+        CLOSED(false);
+
+        private final boolean newStreamsAllowed;
+
+        private ConnectionState(boolean newStreamsAllowed) {
+            this.newStreamsAllowed = newStreamsAllowed;
+        }
+
+        public boolean isNewStreamAllowed() {
+            return newStreamsAllowed;
+        }
+     }
 }

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=1688655&r1=1688654&r2=1688655&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties Wed Jul  1 13:53:52 2015
@@ -61,6 +61,8 @@ http2Parser.processFrameWindowUpdate.inv
 http2Parser.processFrameWindowUpdate.invalidPayloadSize=Window update frame received with an invalid payload size of [{0}]
 http2Parser.swallow.debug=Connection [{0}], Stream [{1}], Swallowed [{2}] bytes
 
+pingManager.roundTripTime=Connection [{0}] Round trip time measured as [{1}]ns
+
 stream.closed=Connection [{0}], Stream [{1}], Unable to write to stream once it has been closed
 stream.header.debug=Connection [{0}], Stream [{1}], HTTP header [{2}], Value [{3}]
 stream.reprioritisation.debug=Connection [{0}], Stream [{1}], Exclusive [{2}], Parent [{3}], Weight [{4}]
@@ -75,12 +77,14 @@ streamStateMachine.invalidFrame=Connecti
 upgradeHandler.allocate.debug=Connection [{0}], Stream [{1}], allocated [{2}] bytes
 upgradeHandler.allocate.left=Connection [{0}], Stream [{1}], [{2}] bytes unallocated - trying to allocate to children
 upgradeHandler.allocate.recipient=Connection [{0}], Stream [{1}], potential recipient [{2}] with weight [{3}]
+upgradeHandler.noNewStreams=Connection [{0}], Stream [{1}], Stream ignored as no new streams are permitted on this connection
 upgradeHandler.rst.debug=Connection [{0}], Stream [{1}], Error [{2}], RST (closing stream)
 upgradeHandler.goaway.debug=Connection [{0}], Goaway, Last stream [{1}], Error code [{2}], Debug data [{3}]
 upgradeHandler.init=Connection [{0}]
 upgradeHandler.invalidPreface=Connection [{0}], Invalid connection preface
 upgradeHandler.ioerror=Connection [{0}]
 upgradeHandler.pause.entry=Connection [{0}] Pausing
+upgradeHandler.pingFailed=Connection [{0}] Failed to send ping to client
 upgradeHandler.sendPrefaceFail=Failed to send preface to client
 upgradeHandler.socketCloseFailed=Error closing socket
 upgradeHandler.stream.closed=Stream [{0}] has been closed for some time

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=1688655&r1=1688654&r2=1688655&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java (original)
+++ tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java Wed Jul  1 13:53:52 2015
@@ -69,6 +69,12 @@ public abstract class Http2TestBase exte
     protected Http2Parser parser;
     protected OutputStream os;
 
+    private long pingAckDelayMillis = 0;
+
+
+    protected void setPingAckDelayMillis(long delay) {
+        pingAckDelayMillis = delay;
+    }
 
     /**
      * Standard setup. Creates HTTP/2 connection via HTTP upgrade and ensures
@@ -95,9 +101,11 @@ public abstract class Http2TestBase exte
         parser.readFrame(true);
         parser.readFrame(true);
         parser.readFrame(true);
+        parser.readFrame(true);
 
         Assert.assertEquals("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();
@@ -489,6 +497,13 @@ public abstract class Http2TestBase exte
 
 
     void sendPing(int streamId, boolean ack, byte[] payload) throws IOException {
+        if (ack && pingAckDelayMillis > 0) {
+            try {
+                Thread.sleep(pingAckDelayMillis);
+            } catch (InterruptedException e) {
+                // Ignore
+            }
+        }
         byte[] pingHeader = new byte[9];
         // length
         ByteUtil.setThreeBytes(pingHeader, 0, payload.length);
@@ -643,7 +658,7 @@ public abstract class Http2TestBase exte
     }
 
 
-    static class TestOutput implements Output, HeaderEmitter {
+    class TestOutput implements Output, HeaderEmitter {
 
         private StringBuffer trace = new StringBuffer();
         private String lastStreamId = "0";
@@ -722,10 +737,12 @@ public abstract class Http2TestBase exte
 
 
         @Override
-        public void pingReceive(byte[] payload, boolean ack) {
+        public void pingReceive(byte[] payload, boolean ack) throws IOException {
             trace.append("0-Ping-");
             if (ack) {
                 trace.append("Ack-");
+            } else {
+                sendPing(0, true, payload);
             }
             trace.append('[');
             boolean first = true;

Added: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_8.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_8.java?rev=1688655&view=auto
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_8.java (added)
+++ tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_8.java Wed Jul  1 13:53:52 2015
@@ -0,0 +1,84 @@
+/*
+ *  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 org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * Unit tests for Section 6.8 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_8 extends Http2TestBase {
+
+    private static final long PNG_ACK_DELAY_MS = 1000;
+
+    @Test
+    public void testGoawayIgnoreNewStreams() throws Exception {
+        setPingAckDelayMillis(PNG_ACK_DELAY_MS);
+
+        // HTTP2 upgrade
+        http2Connect();
+
+        getTomcatInstance().getConnector().pause();
+
+        // Go away
+        parser.readFrame(true);
+        Assert.assertEquals("0-Goaway-[2147483647]-[0]-[null]", output.getTrace());
+        output.clearTrace();
+
+        // Should be processed
+        sendSimpleGetRequest(3);
+
+        Thread.sleep(PNG_ACK_DELAY_MS + 200);
+
+        // Should be ignored
+        sendSimpleGetRequest(5);
+
+        parser.readFrame(true);
+        parser.readFrame(true);
+
+        Assert.assertEquals(getSimpleResponseTrace(3),  output.getTrace());
+        output.clearTrace();
+
+        // Finally the go away frame
+        parser.readFrame(true);
+        Assert.assertEquals("0-Goaway-[3]-[0]-[null]", output.getTrace());
+    }
+
+
+    @Test
+    public void testGoawayFrameNonZeroStream() throws Exception {
+        // HTTP2 upgrade
+        http2Connect();
+
+        sendGoaway(1, 1, Http2Error.NO_ERROR.getCode(), null);
+
+        // Go away
+        parser.readFrame(true);
+
+        Assert.assertTrue(output.getTrace(), output.getTrace().startsWith(
+                "0-Goaway-[1]-[" + Http2Error.PROTOCOL_ERROR.getCode() + "]-["));
+    }
+
+
+    // TODO Test header processing and window size processing for ignored
+    //      streams
+}

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



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