Partially revert previous error handling changes. Generally, the rese…

…t always needs to be sent (even if the state doesn't change). git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1708580 13f79535-47bb-0310-9956-ffa450edef68
apache · Oct 14, 2015 · 7cb9b48 · 7cb9b48
1 parent d03f592
commit 7cb9b48
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 37 deletions.
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -401,35 +401,28 @@ private void checkPauseState() throws IOException {
     }
 
 
-    void resetStream(StreamException se) throws ConnectionException, IOException {
+    void resetStream(StreamException se) throws IOException {
 
         if (log.isDebugEnabled()) {
             log.debug(sm.getString("upgradeHandler.rst.debug", connectionId,
                     Integer.toString(se.getStreamId()), se.getError()));
         }
 
-        // If the stream is null, the server knows nothing about it. The ID can
-        // only have come from the client so always send a reset.
-        // If the stream is not null, the server does know about the stream so
-        // only send the reset if the stream is in an appropriate state.
-        Stream stream = getStream(se.getStreamId(), false);
-        if (stream == null || stream.sendReset()) {
-            // Write a RST frame
-            byte[] rstFrame = new byte[13];
-            // Length
-            ByteUtil.setThreeBytes(rstFrame, 0, 4);
-            // Type
-            rstFrame[3] = FrameType.RST.getIdByte();
-            // No flags
-            // Stream ID
-            ByteUtil.set31Bits(rstFrame, 5, se.getStreamId());
-            // Payload
-            ByteUtil.setFourBytes(rstFrame, 9, se.getError().getCode());
+        // Write a RST frame
+        byte[] rstFrame = new byte[13];
+        // Length
+        ByteUtil.setThreeBytes(rstFrame, 0, 4);
+        // Type
+        rstFrame[3] = FrameType.RST.getIdByte();
+        // No flags
+        // Stream ID
+        ByteUtil.set31Bits(rstFrame, 5, se.getStreamId());
+        // Payload
+        ByteUtil.setFourBytes(rstFrame, 9, se.getError().getCode());
 
-            synchronized (socketWrapper) {
-                socketWrapper.write(true, rstFrame, 0, rstFrame.length);
-                socketWrapper.flush(true);
-            }
+        synchronized (socketWrapper) {
+            socketWrapper.write(true, rstFrame, 0, rstFrame.length);
+            socketWrapper.flush(true);
         }
     }
 

diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java
@@ -321,14 +321,11 @@ StreamOutputBuffer getOutputBuffer() {
      * <li>The stream is already reset</li>
      * <li>The stream is already closed</li>
      *
-     * @return <code>true</code> if a reset frame needs to be sent to the peer,
-     *         otherwise <code>false</code>
-     *
      * @throws IllegalStateException If the stream is in a state that does not
      *         permit resets
      */
-    boolean sendReset() {
-        return state.sendReset();
+    void sendReset() {
+        state.sendReset();
     }
 
 
@@ -356,8 +353,6 @@ void close(Http2Exception http2Exception) {
         if (http2Exception instanceof StreamException) {
             try {
                 handler.resetStream((StreamException) http2Exception);
-            } catch (ConnectionException ce) {
-                handler.closeConnection(ce);
             } catch (IOException ioe) {
                 // TODO i18n
                 ConnectionException ce = new ConnectionException("", Http2Error.PROTOCOL_ERROR);

diff --git a/java/org/apache/coyote/http2/StreamStateMachine.java b/java/org/apache/coyote/http2/StreamStateMachine.java
@@ -90,22 +90,16 @@ public synchronized void recievedEndOfStream() {
      * <li>The stream is already closed</li>
      * </ul>
      *
-     * @return <code>true</code> if a reset frame needs to be sent to the peer,
-     *         otherwise <code>false</code>
-     *
      * @throws IllegalStateException If the stream is in a state that does not
      *         permit resets
      */
-    public synchronized boolean sendReset() {
+    public synchronized void sendReset() {
         if (state == State.IDLE) {
             throw new IllegalStateException(sm.getString("streamStateMachine.debug.change",
                     stream.getConnectionId(), stream.getIdentifier(), state));
         }
         if (state.canReset()) {
             stateChange(state, State.CLOSED_RST_TX);
-            return true;
-        } else {
-            return false;
         }
     }
 
@@ -217,7 +211,7 @@ private enum State {
                                                        FrameType.RST,
                                                        FrameType.PUSH_PROMISE,
                                                        FrameType.WINDOW_UPDATE),
-        CLOSED_FINAL       (false, false, false, false,
+        CLOSED_FINAL       (false, false, false, true,
                             Http2Error.PROTOCOL_ERROR, FrameType.PRIORITY);
 
         private final boolean canRead;