Skip to content

Commit 19e109c

Browse files
committed
streams: Prevent RST_STREAM from being sent multiple times
Previously, RST_STREAM would be sent even when the stream closed by state was in SEND_RST_STREAM. This fix instead checks which peer closed the stream initially, and then updates the closed by value from RECV_RST_STREAM to SEND_RST_STREAM after a RST_STREAM frame has been sent on a previously reset stream. Streams that have a closed by value of SEND_RST_STREAM now ignore all frames.
1 parent 53feb0e commit 19e109c

File tree

1 file changed

+83
-13
lines changed

1 file changed

+83
-13
lines changed

Diff for: src/h2/connection.py

+83-13
Original file line numberDiff line numberDiff line change
@@ -1486,15 +1486,58 @@ def _receive_frame(self, frame):
14861486
# I don't love using __class__ here, maybe reconsider it.
14871487
frames, events = self._frame_dispatch_table[frame.__class__](frame)
14881488
except StreamClosedError as e:
1489-
# If the stream was closed by RST_STREAM, we just send a RST_STREAM
1490-
# to the remote peer. Otherwise, this is a connection error, and so
1491-
# we will re-raise to trigger one.
1489+
# RFC 7540: Section 5.1 "Stream States" (Relevant portions):
1490+
# closed:
1491+
#
1492+
# ...
1493+
#
1494+
# An endpoint MUST NOT send frames other than PRIORITY on a
1495+
# closed stream. An endpoint that receives any frame other
1496+
# than PRIORITY after receiving a RST_STREAM MUST treat that
1497+
# as a stream error (Section 5.4.2) of type STREAM_CLOSED.
1498+
#
1499+
# ...
1500+
#
1501+
# An endpoint MUST ignore frames that it receives on closed
1502+
# streams after it has sent a RST_STREAM frame.
1503+
1504+
# From the RFC, there are outcomes to consider here:
1505+
# 1) We received a frame on a closed stream that we have not yet
1506+
# sent a RST_STREAM frame on.
1507+
# Action: Send RST_STREAM with error STREAM_CLOSED. An
1508+
# additional consideration here is that after the RST_STREAM has
1509+
# been sent, additional RST_STREAM frames can no longer be sent
1510+
# on this stream. To account for this, update the stream closed
1511+
# reason from RECV_RST_STREAM to SEND_RST_STREAM as we have now
1512+
# sent a RST_STREAM frame.
1513+
# 2) We received a frame on a closed stream which we have already
1514+
# sent a RST_STREAM frame on.
1515+
# Action: Ignore (Illegal to send an additional RST_STREAM)
1516+
# 3) We received a frame on a closed stream that the remote peer
1517+
# agrees was closed. (e.g, stream wasn't closed via RST_STREAM).
1518+
# Action: Protocal Error (re-raise)
1519+
14921520
if self._stream_is_closed_by_reset(e.stream_id):
1493-
f = RstStreamFrame(e.stream_id)
1494-
f.error_code = e.error_code
1495-
self._prepare_for_sending([f])
1496-
events = e._events
1521+
if self._stream_is_closed_by_peer_reset(e.stream_id):
1522+
# This outcome 1) from above
1523+
# Send a RST_STREAM frame and update close reason.
1524+
1525+
f = RstStreamFrame(e.stream_id)
1526+
f.error_code = e.error_code
1527+
self._prepare_for_sending([f])
1528+
events = e._events
1529+
1530+
self._stream_set_closed_by(
1531+
e.stream_id,
1532+
StreamClosedBy.SEND_RST_STREAM
1533+
)
1534+
else:
1535+
# This outcome 2) from above
1536+
# Ignore this frame
1537+
pass
14971538
else:
1539+
# This is outcome 3) from above
1540+
# Re-raise
14981541
raise
14991542
except StreamIDTooLowError as e:
15001543
# The stream ID seems invalid. This may happen when the closed
@@ -1503,13 +1546,21 @@ def _receive_frame(self, frame):
15031546
# closed implicitly.
15041547
#
15051548
# Check how the stream was closed: depending on the mechanism, it
1506-
# is either a stream error or a connection error.
1549+
# is either a stream error or a connection error. This has the
1550+
# same considerations as StreamClosedError when the stream was
1551+
# closed via reset.
15071552
if self._stream_is_closed_by_reset(e.stream_id):
1508-
# Closed by RST_STREAM is a stream error.
1509-
f = RstStreamFrame(e.stream_id)
1510-
f.error_code = ErrorCodes.STREAM_CLOSED
1511-
self._prepare_for_sending([f])
1512-
events = []
1553+
if self._stream_is_closed_by_peer_reset(e.stream_id):
1554+
# This outcome 1) from above
1555+
# Send a RST_STREAM frame and update close reason.
1556+
f = RstStreamFrame(e.stream_id)
1557+
f.error_code = ErrorCodes.STREAM_CLOSED
1558+
self._prepare_for_sending([f])
1559+
events = []
1560+
else:
1561+
# This outcome 2) from above
1562+
# Ignore this frame
1563+
pass
15131564
elif self._stream_is_closed_by_end(e.stream_id):
15141565
# Closed by END_STREAM is a connection error.
15151566
raise StreamClosedError(e.stream_id)
@@ -1976,6 +2027,25 @@ def _stream_is_closed_by_reset(self, stream_id):
19762027
StreamClosedBy.RECV_RST_STREAM, StreamClosedBy.SEND_RST_STREAM
19772028
)
19782029

2030+
def _stream_is_closed_by_peer_reset(self, stream_id):
2031+
"""
2032+
Returns ``True`` if the stream was closed by sending or receiving a
2033+
RST_STREAM frame. Returns ``False`` otherwise.
2034+
"""
2035+
return self._stream_closed_by(stream_id) in (
2036+
StreamClosedBy.RECV_RST_STREAM
2037+
)
2038+
2039+
def _stream_set_closed_by(self, stream_id, closed_by):
2040+
"""
2041+
Updates a streams closed_by value.
2042+
"""
2043+
2044+
if stream_id in self.streams:
2045+
self.streams[stream_id].closed_by = closed_by
2046+
if stream_id in self._closed_streams:
2047+
self._closed_streams[stream_id] = closed_by
2048+
19792049
def _stream_is_closed_by_end(self, stream_id):
19802050
"""
19812051
Returns ``True`` if the stream was closed by sending or receiving an

0 commit comments

Comments
 (0)