[tomcat] 03/03: Workaround https://bz.apache.org/bugzilla/show_bug.cgi?id=63690
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
commit fbbbfc03c8b9b26dff68c19fe9d8f5f95303928a
Author: Mark Thomas
AuthorDate: Thu Sep 5 12:27:24 2019 +0100
Workaround https://bz.apache.org/bugzilla/show_bug.cgi?id=63690
Use the average of the current and previous sizes when calculating
overhead for HTTP/2 DATA and WINDOW_UPDATE frames to avoid false
positives as a result of client side buffering behaviour that causes a
small percentage of non-final DATA frames to be smaller than expected.
---
.../apache/coyote/http2/Http2UpgradeHandler.java | 45 --
webapps/docs/changelog.xml | 7
webapps/docs/config/http2.xml | 29 +++---
3 files changed, 55 insertions(+), 26 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index adeafa9..0b7c05a 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -156,6 +156,8 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
// Track 'overhead' frames vs 'request/response' frames
private final AtomicLong overheadCount = new AtomicLong(-10);
+private volatile int lastNonFinalDataPayload;
+private volatile int lastWindowUpdate;
/**
@@ -176,6 +178,9 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
this.adapter = adapter;
this.connectionId =
Integer.toString(connectionIdGenerator.getAndIncrement());
+lastNonFinalDataPayload = protocol.getOverheadDataThreshold() * 2;
+lastWindowUpdate = protocol.getOverheadWindowUpdateThreshold() * 2;
+
remoteSettings = new ConnectionSettingsRemote(connectionId);
localSettings = new ConnectionSettingsLocal(connectionId);
@@ -1492,15 +1497,21 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
// .. but lots of small payloads are inefficient so that will increase
// the overhead count unless it is the final DATA frame where small
// payloads are expected.
+
+// See also https://bz.apache.org/bugzilla/show_bug.cgi?id=63690
+// The buffering behaviour of some clients means that small data frames
+// are much more frequent (roughly 1 in 20) than expected. Use an
+// average over two frames to avoid false positives.
if (!endOfStream) {
int overheadThreshold = protocol.getOverheadDataThreshold();
-if (payloadSize < overheadThreshold) {
-if (payloadSize == 0) {
-// Avoid division by zero
-overheadCount.addAndGet(overheadThreshold);
-} else {
-overheadCount.addAndGet(overheadThreshold / payloadSize);
-}
+int average = (lastNonFinalDataPayload >> 1) + (payloadSize >> 1);
+lastNonFinalDataPayload = payloadSize;
+// Avoid division by zero
+if (average == 0) {
+average = 1;
+}
+if (average < overheadThreshold) {
+overheadCount.addAndGet(overheadThreshold / average);
}
}
@@ -1735,13 +1746,25 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
@Override
public void incrementWindowSize(int streamId, int increment) throws
Http2Exception {
+// See also https://bz.apache.org/bugzilla/show_bug.cgi?id=63690
+// The buffering behaviour of some clients means that small data frames
+// are much more frequent (roughly 1 in 20) than expected. Some clients
+// issue a Window update for every DATA frame so a similar pattern may
+// be observed. Use an average over two frames to avoid false
positives.
+
+int average = (lastWindowUpdate >> 1) + (increment >> 1);
int overheadThreshold = protocol.getOverheadWindowUpdateThreshold();
+lastWindowUpdate = increment;
+// Avoid division by zero
+if (average == 0) {
+average = 1;
+}
if (streamId == 0) {
// Check for small increments which are inefficient
-if (increment < overheadThreshold) {
+if (average < overheadThreshold) {
// The smaller the increment, the larger the overhead
-overheadCount.addAndGet(overheadThreshold / increment);
+overheadCount.addAndGet(overheadThreshold / average);
}
incrementWindowSize(increment);
@@ -1749,13 +1772,13 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
Stream stream = getStream(streamId, tr
[tomcat] 03/03: Workaround https://bz.apache.org/bugzilla/show_bug.cgi?id=63690
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 3a5556feff7e2be96e53c630f3c3e73a31adc975
Author: Mark Thomas
AuthorDate: Thu Sep 5 12:27:24 2019 +0100
Workaround https://bz.apache.org/bugzilla/show_bug.cgi?id=63690
Use the average of the current and previous sizes when calculating
overhead for HTTP/2 DATA and WINDOW_UPDATE frames to avoid false
positives as a result of client side buffering behaviour that causes a
small percentage of non-final DATA frames to be smaller than expected.
---
.../apache/coyote/http2/Http2UpgradeHandler.java | 45 --
webapps/docs/changelog.xml | 7
webapps/docs/config/http2.xml | 29 +++---
3 files changed, 55 insertions(+), 26 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 9753ebc..10a1f65 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -142,6 +142,8 @@ class Http2UpgradeHandler extends AbstractStream implements
InternalHttpUpgradeH
// Track 'overhead' frames vs 'request/response' frames
private final AtomicLong overheadCount = new AtomicLong(-10);
+private volatile int lastNonFinalDataPayload;
+private volatile int lastWindowUpdate;
Http2UpgradeHandler(Http2Protocol protocol, Adapter adapter, Request
coyoteRequest) {
@@ -150,6 +152,9 @@ class Http2UpgradeHandler extends AbstractStream implements
InternalHttpUpgradeH
this.adapter = adapter;
this.connectionId =
Integer.toString(connectionIdGenerator.getAndIncrement());
+lastNonFinalDataPayload = protocol.getOverheadDataThreshold() * 2;
+lastWindowUpdate = protocol.getOverheadWindowUpdateThreshold() * 2;
+
remoteSettings = new ConnectionSettingsRemote(connectionId);
localSettings = new ConnectionSettingsLocal(connectionId);
@@ -1372,15 +1377,21 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
// .. but lots of small payloads are inefficient so that will increase
// the overhead count unless it is the final DATA frame where small
// payloads are expected.
+
+// See also https://bz.apache.org/bugzilla/show_bug.cgi?id=63690
+// The buffering behaviour of some clients means that small data frames
+// are much more frequent (roughly 1 in 20) than expected. Use an
+// average over two frames to avoid false positives.
if (!endOfStream) {
int overheadThreshold = protocol.getOverheadDataThreshold();
-if (payloadSize < overheadThreshold) {
-if (payloadSize == 0) {
-// Avoid division by zero
-overheadCount.addAndGet(overheadThreshold);
-} else {
-overheadCount.addAndGet(overheadThreshold / payloadSize);
-}
+int average = (lastNonFinalDataPayload >> 1) + (payloadSize >> 1);
+lastNonFinalDataPayload = payloadSize;
+// Avoid division by zero
+if (average == 0) {
+average = 1;
+}
+if (average < overheadThreshold) {
+overheadCount.addAndGet(overheadThreshold / average);
}
}
@@ -1615,13 +1626,25 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
@Override
public void incrementWindowSize(int streamId, int increment) throws
Http2Exception {
+// See also https://bz.apache.org/bugzilla/show_bug.cgi?id=63690
+// The buffering behaviour of some clients means that small data frames
+// are much more frequent (roughly 1 in 20) than expected. Some clients
+// issue a Window update for every DATA frame so a similar pattern may
+// be observed. Use an average over two frames to avoid false
positives.
+
+int average = (lastWindowUpdate >> 1) + (increment >> 1);
int overheadThreshold = protocol.getOverheadWindowUpdateThreshold();
+lastWindowUpdate = increment;
+// Avoid division by zero
+if (average == 0) {
+average = 1;
+}
if (streamId == 0) {
// Check for small increments which are inefficient
-if (increment < overheadThreshold) {
+if (average < overheadThreshold) {
// The smaller the increment, the larger the overhead
-overheadCount.addAndGet(overheadThreshold / increment);
+overheadCount.addAndGet(overheadThreshold / average);
}
incrementWindowSize(increment);
@@ -1629,13 +1652,13 @@ class Http2UpgradeHandler extends AbstractStream
