bryancall commented on code in PR #13833:
URL: https://github.com/apache/trafficserver/pull/13833#discussion_r4263949647
##########
tests/gold_tests/h2/http2_rst_stream.test.py:
##########
@@ -18,12 +18,25 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+import re
+
Test.Summary = '''
Abort HTTP/2 connection using RST_STREAM frame.
'''
Test.SkipUnless(Condition.HasOpenSSLVersion('1.1.1'),
Condition.HasProxyVerifierVersion('2.8.0'))
+
+# When the client resets its stream, the origin sees the request only if ATS
finished
+# its TLS handshake with the origin first. In that case the origin answers,
the response
+# has nowhere to go, and ATS resets the origin stream with NO_ERROR after the
origin has
+# already closed it. That is fine. The client's abort must never reset an
origin stream
+# that is still open, which the origin logs as an RST_STREAM followed by the
stream closing.
+def origin_stream_not_reset():
+ return Testers.ExcludesExpression(
+ 'Received RST_STREAM frame.*HTTP/2 stream is closed', 'Server is not
affected.', reflags=re.DOTALL | re.MULTILINE)
Review Comment:
Fixed in 4407c6097b. The check now requires the reset and the close to name
the same stream id. Each run has a single transaction, so there is only one
origin stream to match, and `.*?` would not change whether a search finds a
match, but tying the id makes the intent explicit. I re-ran it against the
origin logs I kept from earlier runs, about 1,400 of them, and it gives the
same result on every one. It still fails when ATS is built to pass a client
abort through to the origin.
##########
tests/gold_tests/h2/http2_rst_stream.test.py:
##########
@@ -18,12 +18,25 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+import re
+
Test.Summary = '''
Abort HTTP/2 connection using RST_STREAM frame.
'''
Test.SkipUnless(Condition.HasOpenSSLVersion('1.1.1'),
Condition.HasProxyVerifierVersion('2.8.0'))
+
+# When the client resets its stream, the origin sees the request only if ATS
finished
+# its TLS handshake with the origin first. In that case the origin answers,
the response
+# has nowhere to go, and ATS resets the origin stream with NO_ERROR after the
origin has
+# already closed it. That is fine. The client's abort must never reset an
origin stream
+# that is still open, which the origin logs as an RST_STREAM followed by the
stream closing.
+def origin_stream_not_reset():
+ return Testers.ExcludesExpression(
Review Comment:
Won't fix. It is used as `server.Streams.All += origin_stream_not_reset()`,
which reads as the expectation being added, the same way the `Testers` calls
next to it do.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
