You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2021/06/15 07:01:52 UTC
[tomcat] branch main updated: Fix problems with file uploads and
HTTP/2
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 2538f6d Fix problems with file uploads and HTTP/2
2538f6d is described below
commit 2538f6d74949751901afc5e04014ede4f867152a
Author: Mark Thomas <ma...@apache.org>
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.
---
.../coyote/http2/Http2AsyncUpgradeHandler.java | 30 ++++++++++++-------
.../apache/coyote/http2/Http2UpgradeHandler.java | 34 +++++++++++++++-------
.../apache/coyote/http2/LocalStrings.properties | 2 ++
java/org/apache/coyote/http2/Stream.java | 24 +++++++++++++++
webapps/docs/changelog.xml | 10 +++++++
5 files changed, 79 insertions(+), 21 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
index b77fee6..6d7e3eb 100644
--- a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
@@ -231,22 +231,32 @@ public class Http2AsyncUpgradeHandler extends Http2UpgradeHandler {
@Override
void writeWindowUpdate(AbstractNonZeroStream stream, int increment, boolean applicationInitiated)
throws IOException {
+ if (log.isDebugEnabled()) {
+ log.debug(sm.getString("upgradeHandler.windowUpdateConnection",
+ getConnectionId(), Integer.valueOf(increment)));
+ }
// Build window update frame for stream 0
byte[] frame = new byte[13];
ByteUtil.setThreeBytes(frame, 0, 4);
frame[3] = FrameType.WINDOW_UPDATE.getIdByte();
ByteUtil.set31Bits(frame, 9, increment);
// No need to send update from closed stream
- if (stream instanceof Stream && ((Stream) stream).canWrite()) {
- // Change stream Id
- byte[] frame2 = new byte[13];
- ByteUtil.setThreeBytes(frame2, 0, 4);
- frame2[3] = FrameType.WINDOW_UPDATE.getIdByte();
- ByteUtil.set31Bits(frame2, 9, increment);
- ByteUtil.set31Bits(frame2, 5, stream.getIdAsInt());
- socketWrapper.write(BlockingMode.SEMI_BLOCK, protocol.getWriteTimeout(),
- TimeUnit.MILLISECONDS, null, SocketWrapperBase.COMPLETE_WRITE, errorCompletion,
- ByteBuffer.wrap(frame), ByteBuffer.wrap(frame2));
+ 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)));
+ }
+ byte[] frame2 = new byte[13];
+ ByteUtil.setThreeBytes(frame2, 0, 4);
+ frame2[3] = FrameType.WINDOW_UPDATE.getIdByte();
+ ByteUtil.set31Bits(frame2, 9, streamIncrement);
+ ByteUtil.set31Bits(frame2, 5, stream.getIdAsInt());
+ socketWrapper.write(BlockingMode.SEMI_BLOCK, protocol.getWriteTimeout(),
+ TimeUnit.MILLISECONDS, null, SocketWrapperBase.COMPLETE_WRITE, errorCompletion,
+ ByteBuffer.wrap(frame), ByteBuffer.wrap(frame2));
+ }
} else {
socketWrapper.write(BlockingMode.SEMI_BLOCK, protocol.getWriteTimeout(),
TimeUnit.MILLISECONDS, null, SocketWrapperBase.COMPLETE_WRITE, errorCompletion,
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 994a7cb..fa650b7 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -804,6 +804,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];
@@ -812,17 +816,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 3e114d8..25858cb 100644
--- a/java/org/apache/coyote/http2/LocalStrings.properties
+++ b/java/org/apache/coyote/http2/LocalStrings.properties
@@ -158,6 +158,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 28ec14e..8d93d98 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -82,6 +82,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);
@@ -744,6 +747,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 41ebbd0..c5dc156 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -112,6 +112,16 @@
connections. Treat them the same way as clean closes of non-TLS
connections rather than as unknown errors. (markt)
</fix>
+ <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">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch main updated: Fix problems with file uploads and
HTTP/2
Posted by Mark Thomas <ma...@apache.org>.
Sigh.
Someone forgot that:
1. Auto-responders should ignore email with the "Precedence: bulk"
header.
2. Auto-responders should track the addresses they have previously
responded to and not send repeat messages (or at least limit them to
around once a day).
This address just won a global unsubscribe from all ASF lists.
Mark
On 15/06/2021 08:02, Trello Support wrote:
> -- Please reply above this line --
>
>
> Hi there,
>
> THIS IS AN AUTOMATED MESSAGE. PLEASE RESUBMIT YOUR MESSAGE AT
> HTTPS://TRELLO.COM/CONTACT [1]
>
> You've emailed support@trello.com [2], which is no longer available
> for support.
>
> Trello Support is still here to support you, the same as before, but
> through that form.
>
> Thanks very much!
>
>
> Links:
> ------
> [1] https://trello.com/contact
> [2] mailto:support@trello.com
>
> All the best,
>
> Trello
> The Trello Team
>
> Need priority support for your business? Check out Trello Business
> Class [1].
> Visit our Trello community [2] to ask other Trello users for how-to's
> or advice.
>
>
> Links:
> ------
> [1]
> https://trello.com/business-class?utm_source=support-email&utm_medium=email&utm_term=footer&utm_campaign=support-business-class
> [2] https://community.atlassian.com/trello
>
>
> > On Tue, Jun 15, 2021 at 3:02 AM EDT, Tomcat Developers List <dev@tomcat.apache.org> wrote:
>>
>> This is an automated email from the ASF dual-hosted git repository.
>>
>> markt pushed a commit to branch main
>> in repository https://gitbox.apache.org/repos/asf/tomcat.git [1]
>>
>> The following commit(s) were added to refs/heads/main by this push:
>> new 2538f6d Fix problems with file uploads and HTTP/2
>> 2538f6d is described below
>>
>> commit 2538f6d74949751901afc5e04014ede4f867152a
>> Author: Mark Thomas <ma...@apache.org>
>> 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.
>> ---
>> .../coyote/http2/Http2AsyncUpgradeHandler.java | 30
>> ++++++++++++-------
>> .../apache/coyote/http2/Http2UpgradeHandler.java | 34
>> +++++++++++++++-------
>> .../apache/coyote/http2/LocalStrings.properties | 2 ++
>> java/org/apache/coyote/http2/Stream.java | 24 +++++++++++++++
>> webapps/docs/changelog.xml | 10 +++++++
>> 5 files changed, 79 insertions(+), 21 deletions(-)
>>
>> diff --git
>> a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
>> b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
>> index b77fee6..6d7e3eb 100644
>> --- a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
>> +++ b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
>> @@ -231,22 +231,32 @@ public class Http2AsyncUpgradeHandler extends
>> Http2UpgradeHandler {
>> @Override
>> void writeWindowUpdate(AbstractNonZeroStream stream, int increment,
>> boolean applicationInitiated)
>> throws IOException {
>> + if (log.isDebugEnabled()) {
>> + log.debug(sm.getString("upgradeHandler.windowUpdateConnection",
>> + getConnectionId(), Integer.valueOf(increment)));
>> + }
>> // Build window update frame for stream 0
>> byte[] frame = new byte[13];
>> ByteUtil.setThreeBytes(frame, 0, 4);
>> frame[3] = FrameType.WINDOW_UPDATE.getIdByte();
>> ByteUtil.set31Bits(frame, 9, increment);
>> // No need to send update from closed stream
>> - if (stream instanceof Stream &
>> - ByteUtil.setThreeBytes(frame2, 0, 4);
>> - frame2[3] = FrameType.WINDOW_UPDATE.getIdByte();
>> - ByteUtil.set31Bits(frame2, 9, increment);
>> - ByteUtil.set31Bits(frame2, 5, stream.getIdAsInt());
>> - socketWrapper.write(BlockingMode.SEMI_BLOCK,
>> protocol.getWriteTimeout(),
>> - TimeUnit.MILLISECONDS, null, SocketWrapperBase.COMPLETE_WRITE,
>> errorCompletion,
>> - ByteBuffer.wrap(frame), ByteBuffer.wrap(frame2));
>> + if (stream instanceof Stream &
>> + if (streamIncrement > 0) {
>> + if (log.isDebugEnabled()) {
>> + log.debug(sm.getString("upgradeHandler.windowUpdateStream",
>> + getConnectionId(), getIdAsString(),
>> Integer.valueOf(streamIncrement)));
>> + }
>> + byte[] frame2 = new byte[13];
>> + ByteUtil.setThreeBytes(frame2, 0, 4);
>> + frame2[3] = FrameType.WINDOW_UPDATE.getIdByte();
>> + ByteUtil.set31Bits(frame2, 9, streamIncrement);
>> + ByteUtil.set31Bits(frame2, 5, stream.getIdAsInt());
>> + socketWrapper.write(BlockingMode.SEMI_BLOCK,
>> protocol.getWriteTimeout(),
>> + TimeUnit.MILLISECONDS, null, SocketWrapperBase.COMPLETE_WRITE,
>> errorCompletion,
>> + ByteBuffer.wrap(frame), ByteBuffer.wrap(frame2));
>> + }
>> } else {
>> socketWrapper.write(BlockingMode.SEMI_BLOCK,
>> protocol.getWriteTimeout(),
>> TimeUnit.MILLISECONDS, null, SocketWrapperBase.COMPLETE_WRITE,
>> errorCompletion,
>> diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
>> b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
>> index 994a7cb..fa650b7 100644
>> --- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
>> +++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
>> @@ -804,6 +804,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];
>> @@ -812,17 +816,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 &
>> - 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 &
>> + 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 3e114d8..25858cb 100644
>> --- a/java/org/apache/coyote/http2/LocalStrings.properties
>> +++ b/java/org/apache/coyote/http2/LocalStrings.properties
>> @@ -158,6 +158,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 28ec14e..8d93d98 100644
>> --- a/java/org/apache/coyote/http2/Stream.java
>> +++ b/java/org/apache/coyote/http2/Stream.java
>> @@ -82,6 +82,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);
>> @@ -744,6 +747,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 41ebbd0..c5dc156 100644
>> --- a/webapps/docs/changelog.xml
>> +++ b/webapps/docs/changelog.xml
>> @@ -112,6 +112,16 @@
>> connections. Treat them the same way as clean closes of non-TLS
>> connections rather than as unknown errors. (markt)
>> </fix>
>> + <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">
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org [2]
>> For additional commands, e-mail: dev-help@tomcat.apache.org [3]
>>
>> Links:
>> ------
>> [1] https://gitbox.apache.org/repos/asf/tomcat.git
>> [2] mailto:dev-unsubscribe@tomcat.apache.org
>> [3] mailto:dev-help@tomcat.apache.org
>>
>>
>>
>>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch main updated: Fix problems with file uploads and HTTP/2
Posted by Trello Support <su...@trello.com>.
-- Please reply above this line --
Hi there,
THIS IS AN AUTOMATED MESSAGE. PLEASE RESUBMIT YOUR MESSAGE AT
HTTPS://TRELLO.COM/CONTACT [1]
You've emailed support@trello.com [2], which is no longer available
for support.
Trello Support is still here to support you, the same as before, but
through that form.
Thanks very much!
Links:
------
[1] https://trello.com/contact
[2] mailto:support@trello.com
All the best,
Trello
The Trello Team
Need priority support for your business? Check out Trello Business
Class [1].
Visit our Trello community [2] to ask other Trello users for how-to's
or advice.
Links:
------
[1]
https://trello.com/business-class?utm_source=support-email&utm_medium=email&utm_term=footer&utm_campaign=support-business-class
[2] https://community.atlassian.com/trello
> On Tue, Jun 15, 2021 at 3:02 AM EDT, Tomcat Developers List <dev@tomcat.apache.org> wrote:
>
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch main
> in repository https://gitbox.apache.org/repos/asf/tomcat.git [1]
>
> The following commit(s) were added to refs/heads/main by this push:
> new 2538f6d Fix problems with file uploads and HTTP/2
> 2538f6d is described below
>
> commit 2538f6d74949751901afc5e04014ede4f867152a
> Author: Mark Thomas <ma...@apache.org>
> 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.
> ---
> .../coyote/http2/Http2AsyncUpgradeHandler.java | 30
> ++++++++++++-------
> .../apache/coyote/http2/Http2UpgradeHandler.java | 34
> +++++++++++++++-------
> .../apache/coyote/http2/LocalStrings.properties | 2 ++
> java/org/apache/coyote/http2/Stream.java | 24 +++++++++++++++
> webapps/docs/changelog.xml | 10 +++++++
> 5 files changed, 79 insertions(+), 21 deletions(-)
>
> diff --git
> a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
> b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
> index b77fee6..6d7e3eb 100644
> --- a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
> +++ b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
> @@ -231,22 +231,32 @@ public class Http2AsyncUpgradeHandler extends
> Http2UpgradeHandler {
> @Override
> void writeWindowUpdate(AbstractNonZeroStream stream, int increment,
> boolean applicationInitiated)
> throws IOException {
> + if (log.isDebugEnabled()) {
> + log.debug(sm.getString("upgradeHandler.windowUpdateConnection",
> + getConnectionId(), Integer.valueOf(increment)));
> + }
> // Build window update frame for stream 0
> byte[] frame = new byte[13];
> ByteUtil.setThreeBytes(frame, 0, 4);
> frame[3] = FrameType.WINDOW_UPDATE.getIdByte();
> ByteUtil.set31Bits(frame, 9, increment);
> // No need to send update from closed stream
> - if (stream instanceof Stream &
> - ByteUtil.setThreeBytes(frame2, 0, 4);
> - frame2[3] = FrameType.WINDOW_UPDATE.getIdByte();
> - ByteUtil.set31Bits(frame2, 9, increment);
> - ByteUtil.set31Bits(frame2, 5, stream.getIdAsInt());
> - socketWrapper.write(BlockingMode.SEMI_BLOCK,
> protocol.getWriteTimeout(),
> - TimeUnit.MILLISECONDS, null, SocketWrapperBase.COMPLETE_WRITE,
> errorCompletion,
> - ByteBuffer.wrap(frame), ByteBuffer.wrap(frame2));
> + if (stream instanceof Stream &
> + if (streamIncrement > 0) {
> + if (log.isDebugEnabled()) {
> + log.debug(sm.getString("upgradeHandler.windowUpdateStream",
> + getConnectionId(), getIdAsString(),
> Integer.valueOf(streamIncrement)));
> + }
> + byte[] frame2 = new byte[13];
> + ByteUtil.setThreeBytes(frame2, 0, 4);
> + frame2[3] = FrameType.WINDOW_UPDATE.getIdByte();
> + ByteUtil.set31Bits(frame2, 9, streamIncrement);
> + ByteUtil.set31Bits(frame2, 5, stream.getIdAsInt());
> + socketWrapper.write(BlockingMode.SEMI_BLOCK,
> protocol.getWriteTimeout(),
> + TimeUnit.MILLISECONDS, null, SocketWrapperBase.COMPLETE_WRITE,
> errorCompletion,
> + ByteBuffer.wrap(frame), ByteBuffer.wrap(frame2));
> + }
> } else {
> socketWrapper.write(BlockingMode.SEMI_BLOCK,
> protocol.getWriteTimeout(),
> TimeUnit.MILLISECONDS, null, SocketWrapperBase.COMPLETE_WRITE,
> errorCompletion,
> diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
> b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
> index 994a7cb..fa650b7 100644
> --- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
> +++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
> @@ -804,6 +804,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];
> @@ -812,17 +816,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 &
> - 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 &
> + 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 3e114d8..25858cb 100644
> --- a/java/org/apache/coyote/http2/LocalStrings.properties
> +++ b/java/org/apache/coyote/http2/LocalStrings.properties
> @@ -158,6 +158,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 28ec14e..8d93d98 100644
> --- a/java/org/apache/coyote/http2/Stream.java
> +++ b/java/org/apache/coyote/http2/Stream.java
> @@ -82,6 +82,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);
> @@ -744,6 +747,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 41ebbd0..c5dc156 100644
> --- a/webapps/docs/changelog.xml
> +++ b/webapps/docs/changelog.xml
> @@ -112,6 +112,16 @@
> connections. Treat them the same way as clean closes of non-TLS
> connections rather than as unknown errors. (markt)
> </fix>
> + <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">
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org [2]
> For additional commands, e-mail: dev-help@tomcat.apache.org [3]
>
> Links:
> ------
> [1] https://gitbox.apache.org/repos/asf/tomcat.git
> [2] mailto:dev-unsubscribe@tomcat.apache.org
> [3] mailto:dev-help@tomcat.apache.org
>
>
>
>