From 7f748eb6bfaba5207c89dbd7d5adf50fae847145 Mon Sep 17 00:00:00 2001
From: Mark Thomas <markt@apache.org>
Date: Tue, 30 Apr 2019 22:18:12 +0100
Subject: [PATCH] Expand HTTP/2 timeout handling to connection window
 exhaustion on write.

https://github.com/apache/tomcat/commit/7f748eb
---
 .../coyote/http2/Http2UpgradeHandler.java     | 32 +++++++++++++++++--
 java/org/apache/coyote/http2/Stream.java      | 27 +++++++++-------
 webapps/docs/changelog.xml                    |  4 +++
 3 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 1d8770a..ab0369a 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -794,7 +794,26 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
                 }
                 if (allocation == 0) {
                     try {
-                        stream.wait();
+                        // Connection level window is empty. Although this
+                        // request is for a stream, use the connection
+                        // timeout
+                        long writeTimeout = protocol.getWriteTimeout();
+                        if (writeTimeout < 0) {
+                            stream.wait();
+                        } else {
+                            stream.wait(writeTimeout);
+                        }
+                        // Has this stream been granted an allocation
+                        int[] value = backLogStreams.get(stream);
+                        if (value[1] == 0) {
+                            // No allocation
+                            // Close the connection. Do this first since
+                            // closing the stream will raise an exception
+                            close();
+                            // Close the stream (in app code so need to
+                            // signal to app stream is closing)
+                            stream.doWriteTimeout();
+                        }
                     } catch (InterruptedException e) {
                         throw new IOException(sm.getString(
                                 "upgradeHandler.windowSizeReservationInterrupted", connectionId,
@@ -985,11 +1004,20 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH


     private void close() {
-        connectionState.set(ConnectionState.CLOSED);
+        ConnectionState previous = connectionState.getAndSet(ConnectionState.CLOSED);
+        if (previous == ConnectionState.CLOSED) {
+            // Already closed
+            return;
+        }
+
         for (Stream stream : streams.values()) {
             // The connection is closing. Close the associated streams as no
             // longer required.
             stream.receiveReset(Http2Error.CANCEL.getCode());
+            // Release any streams waiting for an allocation
+            synchronized (stream) {
+                stream.notifyAll();
+            }
         }
         try {
             socketWrapper.close();
diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java
index 2c4f67e..8b87b12 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -219,17 +219,7 @@ class Stream extends AbstractStream implements HeaderEmitter {
                     }
                     windowSize = getWindowSize();
                     if (windowSize == 0) {
-                        String msg = sm.getString("stream.writeTimeout");
-                        StreamException se = new StreamException(
-                                msg, Http2Error.ENHANCE_YOUR_CALM, getIdentifier().intValue());
-                        // Prevent the application making further writes
-                        streamOutputBuffer.closed = true;
-                        // Prevent Tomcat's error handling trying to write
-                        coyoteResponse.setError();
-                        coyoteResponse.setErrorReported();
-                        // Trigger a reset once control returns to Tomcat
-                        streamOutputBuffer.reset = se;
-                        throw new CloseNowException(msg, se);
+                        doWriteTimeout();
                     }
                 } else {
                     return 0;
@@ -252,6 +242,21 @@ class Stream extends AbstractStream implements HeaderEmitter {
     }


+    void doWriteTimeout() throws CloseNowException {
+        String msg = sm.getString("stream.writeTimeout");
+        StreamException se = new StreamException(
+                msg, Http2Error.ENHANCE_YOUR_CALM, getIdentifier().intValue());
+        // Prevent the application making further writes
+        streamOutputBuffer.closed = true;
+        // Prevent Tomcat's error handling trying to write
+        coyoteResponse.setError();
+        coyoteResponse.setErrorReported();
+        // Trigger a reset once control returns to Tomcat
+        streamOutputBuffer.reset = se;
+        throw new CloseNowException(msg, se);
+    }
+
+
     @Override
     public final void emitHeader(String name, String value) throws HpackException {
         if (log.isDebugEnabled()) {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index a8abf2d..5665df4 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -362,6 +362,10 @@
       <update>
         Update the internal fork of Commons DBCP 2 to 2.4.0. (markt)
       </update>
+      <fix>
+        Expand HTTP/2 timeout handling to include connection window exhaustion
+        on write. (markt)
+      </fix>
     </changelog>
   </subsection>
 </section>
--
2.19.1
