Author: markt
Date: Fri Feb 1 10:28:08 2019
New Revision: 1852699
URL: http://svn.apache.org/viewvc?rev=1852699&view=rev
Log:
Implement a write timeout for individual Streams
Added:
tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Timeouts.java (with
props)
Modified:
tomcat/trunk/java/org/apache/catalina/connector/OutputBuffer.java
tomcat/trunk/java/org/apache/catalina/core/StandardWrapperValve.java
tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties
tomcat/trunk/java/org/apache/coyote/http2/Stream.java
tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java
tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java
Modified: tomcat/trunk/java/org/apache/catalina/connector/OutputBuffer.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/OutputBuffer.java?rev=1852699&r1=1852698&r2=1852699&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/OutputBuffer.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/OutputBuffer.java Fri Feb
1 10:28:08 2019
@@ -33,6 +33,7 @@ import javax.servlet.http.HttpServletRes
import org.apache.catalina.Globals;
import org.apache.coyote.ActionCode;
+import org.apache.coyote.CloseNowException;
import org.apache.coyote.Response;
import org.apache.tomcat.util.buf.C2BConverter;
import org.apache.tomcat.util.res.StringManager;
@@ -326,6 +327,13 @@ public class OutputBuffer extends Writer
// real write to the adapter
try {
coyoteResponse.doWrite(buf);
+ } catch (CloseNowException e) {
+ // Catch this sub-class as it requires specific handling.
+ // Examples where this exception is thrown:
+ // - HTTP/2 stream timeout
+ // Prevent further output for this response
+ closed = true;
+ throw e;
} catch (IOException e) {
// An IOException on a write is almost always due to
// the remote client aborting the request. Wrap this
Modified: tomcat/trunk/java/org/apache/catalina/core/StandardWrapperValve.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardWrapperValve.java?rev=1852699&r1=1852698&r2=1852699&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardWrapperValve.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardWrapperValve.java Fri
Feb 1 10:28:08 2019
@@ -36,6 +36,7 @@ import org.apache.catalina.connector.Cli
import org.apache.catalina.connector.Request;
import org.apache.catalina.connector.Response;
import org.apache.catalina.valves.ValveBase;
+import org.apache.coyote.CloseNowException;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.buf.MessageBytes;
import org.apache.tomcat.util.log.SystemLogHandler;
@@ -201,7 +202,7 @@ final class StandardWrapperValve
}
}
- } catch (ClientAbortException e) {
+ } catch (ClientAbortException | CloseNowException e) {
if (container.getLogger().isDebugEnabled()) {
container.getLogger().debug(sm.getString(
"standardWrapper.serviceException", wrapper.getName(),
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=1852699&r1=1852698&r2=1852699&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties [UTF-8]
(original)
+++ tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties [UTF-8]
Fri Feb 1 10:28:08 2019
@@ -100,6 +100,7 @@ stream.reset.fail=Connection [{0}], Stre
stream.reset.receive=Connection [{0}], Stream [{1}], Reset received due to
[{2}]
stream.reset.send=Connection [{0}], Stream [{1}], Reset sent due to [{2}]
stream.trailerHeader.noEndOfStream=Connection [{0}], Stream [{1}], The trailer
headers did not include the end of stream flag
+stream.writeTimeout=Timeout waiting for client to increase flow control window
to permit stream data to be written
streamProcessor.error.connection=Connection [{0}], Stream [{1}], An error
occurred during processing that was fatal to the connection
streamProcessor.error.stream=Connection [{0}], Stream [{1}], An error occurred
during processing that was fatal to the stream
Modified: tomcat/trunk/java/org/apache/coyote/http2/Stream.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Stream.java?rev=1852699&r1=1852698&r2=1852699&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Stream.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Stream.java Fri Feb 1 10:28:08
2019
@@ -222,7 +222,21 @@ class Stream extends AbstractStream impl
}
try {
if (block) {
- wait();
+ wait(handler.getProtocol().getStreamWriteTimeout());
+ windowSize = getWindowSize();
+ if (windowSize == 0) {
+ String msg = sm.getString("stream.writeTimeout");
+ StreamException se = new StreamException(
+ msg, Http2Error.ENHANCE_YOUR_CALM,
getIdAsInt());
+ // 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);
+ }
} else {
return 0;
}
@@ -232,7 +246,6 @@ class Stream extends AbstractStream impl
// Stream.
throw new IOException(e);
}
- windowSize = getWindowSize();
}
int allocation;
if (windowSize < reservation) {
@@ -672,6 +685,11 @@ class Stream extends AbstractStream impl
}
+ StreamException getResetException() {
+ return streamOutputBuffer.reset;
+ }
+
+
private static void push(final Http2UpgradeHandler handler, final Request
request,
final Stream stream) throws IOException {
if (org.apache.coyote.Constants.IS_SECURITY_ENABLED) {
@@ -724,6 +742,7 @@ class Stream extends AbstractStream impl
private volatile long written = 0;
private int streamReservation = 0;
private volatile boolean closed = false;
+ private volatile StreamException reset = null;
private volatile boolean endOfStreamSent = false;
/* The write methods are synchronized to ensure that only one thread at
@@ -863,9 +882,14 @@ class Stream extends AbstractStream impl
@Override
public final void end() throws IOException {
- closed = true;
- flush(true);
- writeTrailers();
+ if (reset != null) {
+ throw new CloseNowException(reset);
+ }
+ if (!closed) {
+ closed = true;
+ flush(true);
+ writeTrailers();
+ }
}
/**
Modified: tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java?rev=1852699&r1=1852698&r2=1852699&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java Fri Feb 1
10:28:08 2019
@@ -80,10 +80,13 @@ class StreamProcessor extends AbstractPr
log.info(ce.getMessage(), ce);
stream.close(ce);
} else if (!getErrorState().isIoAllowed()) {
- StreamException se = new
StreamException(sm.getString(
- "streamProcessor.error.stream",
stream.getConnectionId(),
- stream.getIdentifier()),
Http2Error.INTERNAL_ERROR,
- stream.getIdentifier().intValue());
+ StreamException se = stream.getResetException();
+ if (se == null) {
+ se = new StreamException(sm.getString(
+ "streamProcessor.error.stream",
stream.getConnectionId(),
+ stream.getIdentifier()),
Http2Error.INTERNAL_ERROR,
+ stream.getIdentifier().intValue());
+ }
stream.close(se);
}
}
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=1852699&r1=1852698&r2=1852699&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java (original)
+++ tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java Fri Feb 1
10:28:08 2019
@@ -493,8 +493,10 @@ public abstract class Http2TestBase exte
Http2Protocol http2Protocol = new Http2Protocol();
// Short timeouts for now. May need to increase these for CI systems.
http2Protocol.setReadTimeout(2000);
- http2Protocol.setKeepAliveTimeout(5000);
http2Protocol.setWriteTimeout(2000);
+ http2Protocol.setKeepAliveTimeout(5000);
+ http2Protocol.setStreamReadTimeout(1000);
+ http2Protocol.setStreamWriteTimeout(1000);
http2Protocol.setMaxConcurrentStreams(maxConcurrentStreams);
connector.addUpgradeProtocol(http2Protocol);
}
Added: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Timeouts.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Timeouts.java?rev=1852699&view=auto
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Timeouts.java (added)
+++ tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Timeouts.java Fri Feb 1
10:28:08 2019
@@ -0,0 +1,73 @@
+/*
+ * 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.Before;
+import org.junit.Test;
+
+public class TestHttp2Timeouts extends Http2TestBase {
+
+ @Override
+ @Before
+ public void http2Connect() throws Exception {
+ super.http2Connect();
+ sendSettings(0, false, new
SettingValue(Setting.INITIAL_WINDOW_SIZE.getId(), 0));
+ }
+
+
+ /*
+ * Simple request won't fill buffer so timeout will occur in Tomcat
internal
+ * code during response completion.
+ */
+ @Test
+ public void testClientWithEmptyWindow() throws Exception {
+ sendSimpleGetRequest(3);
+
+ // Settings
+ parser.readFrame(false);
+ // Headers
+ parser.readFrame(false);
+
+ output.clearTrace();
+
+ parser.readFrame(false);
+ Assert.assertEquals("3-RST-[11]\n", output.getTrace());
+ }
+
+
+ /*
+ * Large request will fill buffer so timeout will occur in application code
+ * during response write (when Tomcat commits the response and flushes the
+ * buffer as a result of the buffer filling).
+ */
+ @Test
+ public void testClientWithEmptyWindowLargeResponse() throws Exception {
+ sendLargeGetRequest(3);
+
+ // Settings
+ parser.readFrame(false);
+ // Headers
+ parser.readFrame(false);
+
+ output.clearTrace();
+
+ parser.readFrame(false);
+ Assert.assertEquals("3-RST-[11]\n", output.getTrace());
+ }
+
+}
Propchange: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Timeouts.java
------------------------------------------------------------------------------
svn:eol-style = native
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
