Merge pull request #1630 from baranowb/UNDERTOW-2343_2.3.x

[UNDERTOW-2343] properly handle zealous RST frames in case client ca…

Author: fl4via

core/src/main/java/io/undertow/protocols/http2/Http2Channel.java

@@ -237,7 +237,7 @@ public class Http2Channel extends AbstractFramedChannel<Http2Channel, AbstractHt
      */
     private volatile int receiveWindowSize;
 
-    private final StreamCache sentRstStreams = new StreamCache();
+    private final StreamCache resetStreamTracker = new StreamCache();
 
 
     public Http2Channel(StreamConnection connectedStreamChannel, String protocol, ByteBufferPool bufferPool, PooledByteBuffer data, boolean clientSide, boolean fromUpgrade, OptionMap settings) {
@@ -470,10 +470,10 @@ protected AbstractHttp2StreamSourceChannel createChannelImpl(FrameHeaderData fra
                     //make sure it exists
                     StreamHolder existing = currentStreams.get(frameParser.streamId);
                     if (existing == null) {
-                        existing = sentRstStreams.find(frameParser.streamId);
+                        existing = resetStreamTracker.find(frameParser.streamId);
                     }
                     if(existing == null || existing.sourceClosed) {
-                        if (existing != null || sentRstStreams.find(frameParser.streamId) == null) {
+                        if (existing != null || resetStreamTracker.find(frameParser.streamId) == null) {
                             sendGoAway(ERROR_PROTOCOL_ERROR);
                         }
                         frameData.close();
@@ -507,7 +507,7 @@ protected AbstractHttp2StreamSourceChannel createChannelImpl(FrameHeaderData fra
 
                 StreamHolder holder = currentStreams.get(frameParser.streamId);
                 if(holder == null) {
-                    holder = sentRstStreams.find(frameParser.streamId);
+                    holder = resetStreamTracker.find(frameParser.streamId);
                     if (holder != null) {
                         holder.sourceChannel = (Http2StreamSourceChannel) channel;
                     } else {
@@ -723,7 +723,7 @@ protected void handleBrokenSinkChannel(Throwable e) {
     @Override
     protected void closeSubChannels() {
         closeSubChannels(currentStreams);
-        closeSubChannels(sentRstStreams.getStreamHolders());
+        closeSubChannels(resetStreamTracker.getStreamHolders());
     }
 
     private void closeSubChannels(Map<Integer, StreamHolder> streams) {
@@ -859,7 +859,7 @@ public void handleWindowUpdate(int streamId, int deltaWindowSize) throws IOExcep
             StreamHolder holder = currentStreams.get(streamId);
             Http2StreamSinkChannel stream = holder != null ? holder.sinkChannel : null;
             if (stream == null) {
-                if (sentRstStreams.find(streamId) == null && isIdle(streamId)) {
+                if (resetStreamTracker.find(streamId) == null && isIdle(streamId)) {
                     sendGoAway(ERROR_PROTOCOL_ERROR);
                 }
             } else {
@@ -1211,7 +1211,7 @@ public void sendRstStream(int streamId, int statusCode) {
             //no point sending if the channel is closed
             return;
         }
-        sentRstStreams.store(streamId, handleRstStream(streamId, false));
+        handleRstStream(streamId, false);
         if(UndertowLogger.REQUEST_IO_LOGGER.isDebugEnabled()) {
             UndertowLogger.REQUEST_IO_LOGGER.debugf(new ClosedChannelException(), "Sending rststream on channel %s stream %s", this, streamId);
         }
@@ -1222,6 +1222,7 @@ public void sendRstStream(int streamId, int statusCode) {
     private StreamHolder handleRstStream(int streamId, boolean receivedRst) {
         final StreamHolder holder = currentStreams.remove(streamId);
         if(holder != null) {
+            resetStreamTracker.store(streamId, holder);
             if(streamId % 2 == (isClient() ? 1 : 0)) {
                 sendConcurrentStreamsAtomicUpdater.getAndDecrement(this);
             } else {
@@ -1234,24 +1235,46 @@ private StreamHolder handleRstStream(int streamId, boolean receivedRst) {
                 holder.sourceChannel.rstStream();
             }
             if (receivedRst) {
-                long currentTimeMillis = System.currentTimeMillis();
-                // reset the window tracking
-                if (currentTimeMillis - lastRstFrameMillis >= rstFramesTimeWindow) {
-                    lastRstFrameMillis = currentTimeMillis;
-                    receivedRstFramesPerWindow = 1;
+                if (holder.sinkChannel != null && holder.sourceChannel == null) {
+                    //Server side originated, no input from client other than RST
+                    //this can happen on page refresh when push happens, but client
+                    //still has valid cache entry
+                    holder.resetByPeer = receivedRst;
                 } else {
-                    //
-                    receivedRstFramesPerWindow ++;
-                    if (receivedRstFramesPerWindow > maxRstFramesPerWindow) {
-                        sendGoAway(Http2Channel.ERROR_ENHANCE_YOUR_CALM);
-                        UndertowLogger.REQUEST_IO_LOGGER.debugf("Reached maximum number of rst frames %s during %s ms, sending GO_AWAY 11", maxRstFramesPerWindow, rstFramesTimeWindow);
-                    }
+                    handleRstWindow();
                 }
             }
+        } else if(receivedRst){
+            final StreamHolder resetStream = resetStreamTracker.find(streamId);
+            if(resetStream != null && resetStream.resetByPeer) {
+                //This means other side reset stream at some point.
+                //depending on peer or network latency our frames might be late and
+                //cause other end to flare up with RST, this RST can be safely ignored.
+                //TODO: do we need to check error code?
+            } else {
+                handleRstWindow();
+            }
         }
         return holder;
     }
 
+    private void handleRstWindow() {
+        long currentTimeMillis = System.currentTimeMillis();
+        // reset the window tracking
+        if (currentTimeMillis - lastRstFrameMillis >= rstFramesTimeWindow) {
+            lastRstFrameMillis = currentTimeMillis;
+            receivedRstFramesPerWindow = 1;
+        } else {
+            receivedRstFramesPerWindow++;
+            if (receivedRstFramesPerWindow > maxRstFramesPerWindow) {
+                sendGoAway(Http2Channel.ERROR_ENHANCE_YOUR_CALM);
+                UndertowLogger.REQUEST_IO_LOGGER.debugf(
+                        "Reached maximum number of rst frames %s during %s ms, sending GO_AWAY 11",
+                        maxRstFramesPerWindow, rstFramesTimeWindow);
+            }
+        }
+    }
+
     /**
      * Creates a response stream to respond to the initial HTTP upgrade
      *
@@ -1286,7 +1309,7 @@ public boolean isThisGoneAway() {
     Http2StreamSourceChannel removeStreamSource(int streamId) {
         StreamHolder existing = currentStreams.get(streamId);
         if (existing == null) {
-            existing = sentRstStreams.find(streamId);
+            existing = resetStreamTracker.find(streamId);
             return existing == null? null : existing.sourceChannel;
         }
         existing.sourceClosed = true;
@@ -1306,7 +1329,7 @@ Http2StreamSourceChannel removeStreamSource(int streamId) {
     Http2StreamSourceChannel getIncomingStream(int streamId) {
         StreamHolder existing = currentStreams.get(streamId);
         if(existing == null){
-            existing = sentRstStreams.find(streamId);
+            existing = resetStreamTracker.find(streamId);
             if (existing == null) {
                 return null;
             }
@@ -1351,6 +1374,10 @@ int getMaxHeaderListSize() {
     private static final class StreamHolder {
         boolean sourceClosed = false;
         boolean sinkClosed = false;
+        /**
+         * This flag is set only in case of short lived server push that was reset by remote end.
+         */
+        boolean resetByPeer = false;
         Http2StreamSourceChannel sourceChannel;
         Http2StreamSinkChannel sinkChannel;