Author: markt Date: Mon May 23 11:33:02 2016 New Revision: 1745148 URL: http://svn.apache.org/viewvc?rev=1745148&view=rev Log: More clearly differentiate between resetting the stream and just sending the reset frame. Ensure that the correct approach is taken. This reduces the instance of NPEs when running unit tests as reported by rjung.
Modified: tomcat/tc8.5.x/trunk/ (props changed) tomcat/tc8.5.x/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java tomcat/tc8.5.x/trunk/java/org/apache/coyote/http2/Stream.java tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml Propchange: tomcat/tc8.5.x/trunk/ ------------------------------------------------------------------------------ --- svn:mergeinfo (original) +++ svn:mergeinfo Mon May 23 11:33:02 2016 @@ -1 +1 @@ -/tomcat/trunk:1734785,1734799,1734845,1734928,1735041,1735044,1735480,1735577,1735597,1735599-1735600,1735615,1736145,1736162,1736209,1736280,1736297,1736299,1736489,1736646,1736703,1736836,1736849,1737104-1737105,1737112,1737117,1737119-1737120,1737155,1737157,1737192,1737280,1737339,1737632,1737664,1737715,1737748,1737785,1737834,1737860,1737959,1738005,1738007,1738014-1738015,1738018,1738022,1738039,1738043,1738059-1738060,1738147,1738149,1738174-1738175,1738261,1738589,1738623-1738625,1738643,1738816,1738850,1738855,1738946-1738948,1738953-1738954,1738979,1738982,1739079-1739081,1739087,1739113,1739153,1739172,1739176,1739191,1739474,1739726,1739762,1739775,1739814,1739817-1739818,1739975,1740131,1740324,1740465,1740495,1740508-1740509,1740520,1740535,1740707,1740803,1740810,1740969,1740980,1740991,1740997,1741015,1741033,1741036,1741058,1741060,1741080,1741147,1741159,1741164,1741173,1741181,1741190,1741197,1741202,1741208,1741213,1741221,1741225,1741232,1741409,1741501,1741677 ,1741892,1741896,1741984,1742023,1742042,1742071,1742090,1742093,1742101,1742105,1742111,1742139,1742146,1742148,1742166,1742181,1742184,1742187,1742246,1742248-1742251,1742263-1742264,1742268,1742276,1742369,1742387,1742448,1742509-1742512,1742917,1742919,1742933,1742975-1742976,1742984,1742986,1743019,1743115,1743117,1743124-1743125,1743134,1743425,1743554,1743679,1743696-1743698,1743700-1743701,1744058,1744064-1744065,1744125,1744194,1744229,1744270,1744323,1744432,1744684,1744697,1744705,1744713,1744760,1744786,1745142-1745143 +/tomcat/trunk:1734785,1734799,1734845,1734928,1735041,1735044,1735480,1735577,1735597,1735599-1735600,1735615,1736145,1736162,1736209,1736280,1736297,1736299,1736489,1736646,1736703,1736836,1736849,1737104-1737105,1737112,1737117,1737119-1737120,1737155,1737157,1737192,1737280,1737339,1737632,1737664,1737715,1737748,1737785,1737834,1737860,1737959,1738005,1738007,1738014-1738015,1738018,1738022,1738039,1738043,1738059-1738060,1738147,1738149,1738174-1738175,1738261,1738589,1738623-1738625,1738643,1738816,1738850,1738855,1738946-1738948,1738953-1738954,1738979,1738982,1739079-1739081,1739087,1739113,1739153,1739172,1739176,1739191,1739474,1739726,1739762,1739775,1739814,1739817-1739818,1739975,1740131,1740324,1740465,1740495,1740508-1740509,1740520,1740535,1740707,1740803,1740810,1740969,1740980,1740991,1740997,1741015,1741033,1741036,1741058,1741060,1741080,1741147,1741159,1741164,1741173,1741181,1741190,1741197,1741202,1741208,1741213,1741221,1741225,1741232,1741409,1741501,1741677 ,1741892,1741896,1741984,1742023,1742042,1742071,1742090,1742093,1742101,1742105,1742111,1742139,1742146,1742148,1742166,1742181,1742184,1742187,1742246,1742248-1742251,1742263-1742264,1742268,1742276,1742369,1742387,1742448,1742509-1742512,1742917,1742919,1742933,1742975-1742976,1742984,1742986,1743019,1743115,1743117,1743124-1743125,1743134,1743425,1743554,1743679,1743696-1743698,1743700-1743701,1744058,1744064-1744065,1744125,1744194,1744229,1744270,1744323,1744432,1744684,1744697,1744705,1744713,1744760,1744786,1745142-1745143,1745145 Modified: tomcat/tc8.5.x/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java?rev=1745148&r1=1745147&r2=1745148&view=diff ============================================================================== --- tomcat/tc8.5.x/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java (original) +++ tomcat/tc8.5.x/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java Mon May 23 11:33:02 2016 @@ -294,7 +294,12 @@ public class Http2UpgradeHandler extends } catch (StreamException se) { // Stream errors are not fatal to the connection so // continue reading frames - resetStream(se); + Stream stream = getStream(se.getStreamId(), false); + if (stream == null) { + sendStreamReset(se); + } else { + stream.close(se); + } } } // No more frames to read so switch to the keep-alive @@ -385,7 +390,7 @@ public class Http2UpgradeHandler extends } - void resetStream(StreamException se) throws IOException { + void sendStreamReset(StreamException se) throws IOException { if (log.isDebugEnabled()) { log.debug(sm.getString("upgradeHandler.rst.debug", connectionId, @@ -1249,17 +1254,11 @@ public class Http2UpgradeHandler extends try { stream.incrementWindowSize(diff); } catch (Http2Exception h2e) { - try { - resetStream(new StreamException(sm.getString( - "upgradeHandler.windowSizeTooBig", connectionId, - stream.getIdentifier()), - h2e.getError(), stream.getIdentifier().intValue())); - } catch (IOException ioe) { - if (log.isDebugEnabled()) { - log.debug(sm.getString("upgradeHandler.socketCloseFailed"), ioe); - } - } - } + stream.close(new StreamException(sm.getString( + "upgradeHandler.windowSizeTooBig", connectionId, + stream.getIdentifier()), + h2e.getError(), stream.getIdentifier().intValue())); + } } } else { remoteSettings.set(setting, value); Modified: tomcat/tc8.5.x/trunk/java/org/apache/coyote/http2/Stream.java URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/coyote/http2/Stream.java?rev=1745148&r1=1745147&r2=1745148&view=diff ============================================================================== --- tomcat/tc8.5.x/trunk/java/org/apache/coyote/http2/Stream.java (original) +++ tomcat/tc8.5.x/trunk/java/org/apache/coyote/http2/Stream.java Mon May 23 11:33:02 2016 @@ -369,7 +369,9 @@ public class Stream extends AbstractStre void close(Http2Exception http2Exception) { if (http2Exception instanceof StreamException) { try { - handler.resetStream((StreamException) http2Exception); + StreamException se = (StreamException) http2Exception; + reset(se.getError().getCode()); + handler.sendStreamReset(se); } catch (IOException ioe) { ConnectionException ce = new ConnectionException( sm.getString("stream.reset.fail"), Http2Error.PROTOCOL_ERROR); Modified: tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml?rev=1745148&r1=1745147&r2=1745148&view=diff ============================================================================== --- tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml Mon May 23 11:33:02 2016 @@ -84,6 +84,9 @@ that request processing has fully completed before starting the next request. (markt) </fix> + <fix> + Improve handling of HTTP/2 stream resets. (markt) + </fix> </changelog> </subsection> <subsection name="Jasper"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org