-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deal with RST_STREAM
in HalfClosedLocal
state
#107
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -557,21 +557,32 @@ stream FrameRSTStream header@FrameHeader{streamId} bs ctx s strm = do | |
ConnectionErrorIsSent EnhanceYourCalm streamId "too many rst_stream" | ||
RSTStreamFrame err <- guardIt $ decodeRSTStreamFrame header bs | ||
let cc = Reset err | ||
closed ctx strm cc | ||
|
||
-- The spec mandates (section 8.1): | ||
-- HTTP2 spec, section 5.1, "Stream States": | ||
-- | ||
-- > A stream in the "open" state may be used by both peers to send frames | ||
-- > of any type. (..) From this state, either endpoint can send a frame | ||
-- > with an END_STREAM flag set, which causes the stream to transition into | ||
-- > one of the "half-closed" states. An endpoint sending an END_STREAM | ||
-- > flag causes the stream state to become "half-closed (local)"; an | ||
-- > endpoint receiving an END_STREAM flag causes the stream state to become | ||
-- > "half-closed (remote)". | ||
-- | ||
-- > When this is true, a server MAY request that the client abort | ||
-- > transmission of a request without error by sending a RST_STREAM with an | ||
-- > error code of NO_ERROR after sending a complete response (i.e., a frame | ||
-- > with the END_STREAM flag). | ||
-- Crucially (for the specific case we're dealing with here), it continues: | ||
-- | ||
-- We check the first part ("after sending a complete response") by checking | ||
-- the current stream state. | ||
-- > /Either endpoint/ can send a RST_STREAM frame from this state, causing | ||
-- > it to transition immediately to "closed". | ||
-- | ||
-- (emphasis not in original). This justifies the two non-error cases, | ||
-- below. (Section 8.1 of the spec is also relevant, but it is less explicit | ||
-- about the /either endpoint/ part.) | ||
case (s, err) of | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the case of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense. I guess I introduced that bug as part of #78, apologies. I've pushed a fix, but let me run my full test suite first before merging. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, all tests pass ✔️ |
||
(Open (Just _) _, NoError) -> -- HalfClosedLocal | ||
return (Closed cc) | ||
(HalfClosedRemote, NoError) -> | ||
return (Closed cc) | ||
_otherwise -> do | ||
closed ctx strm cc | ||
E.throwIO $ StreamErrorIsReceived err streamId | ||
|
||
-- (No state transition) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does
(..)
mean?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh,
(..)
is just a standard way of indicating that some text was omitted. In this case, I only showed part of the spec that was relevant.