This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 97ebf75 Fix problems with file uploads and HTTP/2
97ebf75 is described below
commit 97ebf75dd0c617267b7d38386e35ad18637d9e60
Author: Mark Thomas <[email protected]>
AuthorDate: Mon Jun 14 21:15:30 2021 +0100
Fix problems with file uploads and HTTP/2
Small writes from the user agent were triggering small window updates
which in turn triggered more small writes. With lots of small writes the
overhead protection code was activated and the connection closed.
---
.../apache/coyote/http2/Http2UpgradeHandler.java | 34 +++++++++++++++-------
.../apache/coyote/http2/LocalStrings.properties | 2 ++
java/org/apache/coyote/http2/Stream.java | 24 +++++++++++++++
webapps/docs/changelog.xml | 14 +++++++++
4 files changed, 63 insertions(+), 11 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 4c5ab94..3e133df 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -807,6 +807,10 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
*/
void writeWindowUpdate(AbstractNonZeroStream stream, int increment,
boolean applicationInitiated)
throws IOException {
+ if (log.isDebugEnabled()) {
+ log.debug(sm.getString("upgradeHandler.windowUpdateConnection",
+ getConnectionId(), Integer.valueOf(increment)));
+ }
synchronized (socketWrapper) {
// Build window update frame for stream 0
byte[] frame = new byte[13];
@@ -815,17 +819,25 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
ByteUtil.set31Bits(frame, 9, increment);
socketWrapper.write(true, frame, 0, frame.length);
// No need to send update from closed stream
- if (stream instanceof Stream && ((Stream) stream).canWrite()) {
- // Change stream Id and re-use
- ByteUtil.set31Bits(frame, 5, stream.getIdAsInt());
- try {
- socketWrapper.write(true, frame, 0, frame.length);
- socketWrapper.flush(true);
- } catch (IOException ioe) {
- if (applicationInitiated) {
- handleAppInitiatedIOException(ioe);
- } else {
- throw ioe;
+ if (stream instanceof Stream && ((Stream) stream).canWrite()) {
+ int streamIncrement = ((Stream)
stream).getWindowUpdateSizeToWrite(increment);
+ if (streamIncrement > 0) {
+ if (log.isDebugEnabled()) {
+
log.debug(sm.getString("upgradeHandler.windowUpdateStream",
+ getConnectionId(), getIdAsString(),
Integer.valueOf(streamIncrement)));
+ }
+ // Re-use buffer as connection update has already been
written
+ ByteUtil.set31Bits(frame, 5, stream.getIdAsInt());
+ ByteUtil.set31Bits(frame, 9, streamIncrement);
+ try {
+ socketWrapper.write(true, frame, 0, frame.length);
+ socketWrapper.flush(true);
+ } catch (IOException ioe) {
+ if (applicationInitiated) {
+ handleAppInitiatedIOException(ioe);
+ } else {
+ throw ioe;
+ }
}
}
} else {
diff --git a/java/org/apache/coyote/http2/LocalStrings.properties
b/java/org/apache/coyote/http2/LocalStrings.properties
index 2c45055..e0567e2 100644
--- a/java/org/apache/coyote/http2/LocalStrings.properties
+++ b/java/org/apache/coyote/http2/LocalStrings.properties
@@ -157,6 +157,8 @@ upgradeHandler.upgradeDispatch.entry=Entry, Connection
[{0}], SocketStatus [{1}]
upgradeHandler.upgradeDispatch.exit=Exit, Connection [{0}], SocketState [{1}]
upgradeHandler.windowSizeReservationInterrupted=Connection [{0}], Stream
[{1}], reservation for [{2}] bytes
upgradeHandler.windowSizeTooBig=Connection [{0}], Stream [{1}], Window size
too big
+upgradeHandler.windowUpdateConnection=Connection [{0}], Sent window update to
client increasing window by [{1}] bytes
+upgradeHandler.windowUpdateStream=Connection [{0}], Stream [{1}], Sent window
update to client increasing window by [{2}] bytes
upgradeHandler.writeBody=Connection [{0}], Stream [{1}], Data length [{2}],
EndOfStream [{3}]
upgradeHandler.writeHeaders=Connection [{0}], Stream [{1}], Writing the
headers, EndOfStream [{2}]
upgradeHandler.writePushHeaders=Connection [{0}], Stream [{1}], Pushed stream
[{2}], EndOfStream [{3}]
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index becf334..b07b8e2 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -79,6 +79,9 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
private volatile StringBuilder cookieHeader = null;
+ private Object pendingWindowUpdateForStreamLock = new Object();
+ private int pendingWindowUpdateForStream = 0;
+
Stream(Integer identifier, Http2UpgradeHandler handler) {
this(identifier, handler, null);
@@ -696,6 +699,27 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
}
+ int getWindowUpdateSizeToWrite(int increment) {
+ int result;
+ int threshold =
handler.getProtocol().getOverheadWindowUpdateThreshold();
+ synchronized (pendingWindowUpdateForStreamLock) {
+ if (increment > threshold) {
+ result = increment + pendingWindowUpdateForStream;
+ pendingWindowUpdateForStream = 0;
+ } else {
+ pendingWindowUpdateForStream += increment;
+ if (pendingWindowUpdateForStream > threshold) {
+ result = pendingWindowUpdateForStream;
+ pendingWindowUpdateForStream = 0;
+ } else {
+ result = 0;
+ }
+ }
+ }
+ return result;
+ }
+
+
private static void push(final Http2UpgradeHandler handler, final Request
request,
final Stream stream) throws IOException {
if (org.apache.coyote.Constants.IS_SECURITY_ENABLED) {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 084a2fa..1b94d7f 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -105,6 +105,20 @@
issues do not "pop up" wrt. others).
-->
<section name="Tomcat 8.5.68 (schultz)" rtext="in development">
+ <subsection name="Coyote">
+ <changelog>
+ <fix>
+ Modify the HTTP/2 connector not to sent small updates for stream flow
+ control windows to the user agent as, depending on how the user agent
is
+ written, this may trigger small writes from the user agent that in turn
+ trigger the overhead protection. Small updates for stream flow control
+ windows are now combined with subsequent flow control window updates
for
+ that stream to ensure that all stream flow control window updates sent
+ from Tomcat are larger than <code>overheadWindowUpdateThreshold</code>.
+ (markt)
+ </fix>
+ </changelog>
+ </subsection>
<subsection name="Other">
<changelog>
<update>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
