dgrr / http2

HTTP/2 implementation for fasthttp
Apache License 2.0
210 stars 37 forks source link

WINDOW_UPDATE must be ignored on closed connection as stated by RFC 7540 #64

Open pemarko opened 1 year ago

pemarko commented 1 year ago

I see a GO_AWAY with CLOSED_STREAM error sent to client when WINDOS_UPDATE is received right after server sent END_STREAM and closed stream locally. This is seen when using fasthttp2 client which sends WINDOW_UPDATE despite it should already know that server closed stream locally - client has local half-closed stream and just received frame with END_STREAM flag.

RFC says: Endpoints MUST ignore [WINDOW_UPDATE] or [RST_STREAM] frames received in this state, though endpoints MAY choose to treat frames that arrive a significant time after sending END_STREAM as a connection error ([Section 5.4.1] of type [PROTOCOL_ERROR]

Easy fix is to add FrameWindowUpdate type as allowed frames as in below patch. Ideally, timestamp of close event might be tracked to make decision based on age of closed stream.

diff --git a/serverConn.go b/serverConn.go
index cd0e2ea..3df2c5f 100644
--- a/serverConn.go
+++ b/serverConn.go
@@ -374,7 +374,7 @@ loop:
                                }

                                if _, ok := closedStrms[fr.Stream()]; ok {
-                                       if fr.Type() != FramePriority {
+                                       if fr.Type() != FramePriority && fr.Type() != FrameWindowUpdate {
                                                sc.writeGoAway(fr.Stream(), StreamClosedError, "frame on closed stream")
                                        }

Thx,