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 b0d7d1603a Fix BZ 66194 - align HTTP/2 with HTTP/1.1 for handling of client errors b0d7d1603a is described below commit b0d7d1603a8c916469913962af1631a899d8ca05 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 4d43f7c3ef..e9651c510e 100644 --- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java +++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java @@ -49,6 +49,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.SocketEvent; @@ -158,6 +159,8 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH private volatile int lastNonFinalDataPayload; private volatile int lastWindowUpdate; + protected final UserDataHelper userDataHelper = new UserDataHelper(log); + public Http2UpgradeHandler(Http2Protocol protocol, Adapter adapter, Request coyoteRequest) { super (STREAM_ID_ZERO); @@ -345,6 +348,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 32d14f657e..05f2b125bf 100644 --- a/java/org/apache/coyote/http2/LocalStrings.properties +++ b/java/org/apache/coyote/http2/LocalStrings.properties @@ -128,6 +128,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.initialWindowSize.invalid=Connection [{0}], Illegal value of [{1}] ignored for initial window size @@ -149,6 +151,7 @@ upgradeHandler.sendPrefaceFail=Connection [{0}], Failed to send preface to clien 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 d7ab2b5c3d..f956088b6a 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -137,6 +137,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 a3487b4221..aa302c3f8b 100644 --- a/webapps/docs/config/systemprops.xml +++ b/webapps/docs/config/systemprops.xml @@ -486,6 +486,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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org