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 2022/08/23 18:28:44 UTC
[tomcat] branch 10.0.x updated: Fix BZ 66194 - align HTTP/2 with HTTP/1.1 for handling of client errors
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.0.x by this push:
new 60745ec2d9 Fix BZ 66194 - align HTTP/2 with HTTP/1.1 for handling of client errors
60745ec2d9 is described below
commit 60745ec2d9370f205ca7547b7b0d0d22e15b3acf
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Aug 23 19:24:39 2022 +0100
Fix BZ 66194 - align HTTP/2 with HTTP/1.1 for handling of client errors
https://bz.apache.org/bugzilla/show_bug.cgi?id=66194
Also covers exceeding various limits (max header size, cookie count etc)
---
.../org/apache/coyote/http2/Http2UpgradeHandler.java | 20 ++++++++++++++++++++
java/org/apache/coyote/http2/LocalStrings.properties | 3 +++
webapps/docs/changelog.xml | 5 +++++
webapps/docs/config/systemprops.xml | 1 +
4 files changed, 29 insertions(+)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index de33bfded6..068677905e 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -48,6 +48,7 @@ import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.codec.binary.Base64;
import org.apache.tomcat.util.http.MimeHeaders;
+import org.apache.tomcat.util.log.UserDataHelper;
import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
import org.apache.tomcat.util.net.SSLSupport;
import org.apache.tomcat.util.net.SendfileState;
@@ -146,6 +147,8 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
private volatile int lastNonFinalDataPayload;
private volatile int lastWindowUpdate;
+ protected final UserDataHelper userDataHelper = new UserDataHelper(log);
+
Http2UpgradeHandler(Http2Protocol protocol, Adapter adapter, Request coyoteRequest) {
super (STREAM_ID_ZERO);
@@ -351,6 +354,23 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
break;
}
} catch (StreamException se) {
+ // Log the Stream error but not necessarily all of
+ // them
+ UserDataHelper.Mode logMode = userDataHelper.getNextMode();
+ if (logMode != null) {
+ String message = sm.getString("upgradeHandler.stream.error",
+ connectionId, Integer.toString(se.getStreamId()));
+ switch (logMode) {
+ case INFO_THEN_DEBUG:
+ message += sm.getString("upgradeHandler.fallToDebug");
+ //$FALL-THROUGH$
+ case INFO:
+ log.info(message, se);
+ break;
+ case DEBUG:
+ log.debug(message, se);
+ }
+ }
// Stream errors are not fatal to the connection so
// continue reading frames
Stream stream = getStream(se.getStreamId(), false);
diff --git a/java/org/apache/coyote/http2/LocalStrings.properties b/java/org/apache/coyote/http2/LocalStrings.properties
index 098afef83e..615e6413dc 100644
--- a/java/org/apache/coyote/http2/LocalStrings.properties
+++ b/java/org/apache/coyote/http2/LocalStrings.properties
@@ -129,6 +129,8 @@ upgradeHandler.allocate.left=Connection [{0}], Stream [{1}], [{2}] bytes unalloc
upgradeHandler.allocate.recipient=Connection [{0}], Stream [{1}], potential recipient [{2}] with weight [{3}]
upgradeHandler.connectionError=Connection error
upgradeHandler.dependency.invalid=Connection [{0}], Stream [{1}], Streams may not depend on themselves
+upgradeHandler.fallToDebug=\n\
+\ Note: further occurrences of HTTP/2 stream errors will be logged at DEBUG level.
upgradeHandler.goaway.debug=Connection [{0}], Goaway, Last stream [{1}], Error code [{2}], Debug data [{3}]
upgradeHandler.init=Connection [{0}], State [{1}]
upgradeHandler.invalidPreface=Connection [{0}], Invalid connection preface
@@ -151,6 +153,7 @@ upgradeHandler.sendfile.reservation=Connection [{0}], Stream [{1}], Connection r
upgradeHandler.socketCloseFailed=Error closing socket
upgradeHandler.startRequestBodyFrame.result=Connection [{0}], Stream [{1}] startRequestBodyFrame returned [{2}]
upgradeHandler.stream.closed=Stream [{0}] has been closed for some time
+upgradeHandler.stream.error=Connection [{0}], Stream [{1}] Closed due to error
upgradeHandler.stream.even=A new remote stream ID of [{0}] was requested but all remote streams must use odd identifiers
upgradeHandler.stream.notWritable=Connection [{0}], Stream [{1}], This stream is not writable
upgradeHandler.stream.old=A new remote stream ID of [{0}] was requested but the most recent stream was [{1}]
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 94a3b2c783..06c990c816 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -183,6 +183,11 @@
directives will now be ignored rather than triggering a 500 response.
(markt)
</fix>
+ <fix>
+ <bug>66194</bug>: Log HTTP/2 stream closures (usually caused by client
+ errors) via a <code>UserDataHelper</code> to broadly align it with the
+ behaviour of HTTP/1.1 for parsing issues and exceeding limits. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
diff --git a/webapps/docs/config/systemprops.xml b/webapps/docs/config/systemprops.xml
index bf637353ab..809c6fadd0 100644
--- a/webapps/docs/config/systemprops.xml
+++ b/webapps/docs/config/systemprops.xml
@@ -222,6 +222,7 @@
<code>maxHeaderCount</code> or <code>maxParameterCount</code> limits
of a <a href="http.html">connector</a>).</li>
<li>invalid host names</li>
+ <li>HTTP/2 stream closures</li>
</ul>
<p>Other errors triggered by invalid input data may be added to this
system in later versions.</p>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org