http2 5.4.7 → 5.4.8
raw patch · 9 files changed
+180/−142 lines, 9 filesPVP ok
version bump matches the API change (PVP)
API changes (from Hackage documentation)
Files
- ChangeLog.md +63/−118
- Network/HTTP2/Client/Run.hs +1/−1
- Network/HTTP2/H2/Config.hs +8/−3
- Network/HTTP2/H2/Sender.hs +15/−9
- Network/HTTP2/H2/Sync.hs +6/−2
- Network/HTTP2/Server/Worker.hs +3/−3
- http2.cabal +1/−1
- test/HTTP2/FrameSpec.hs +4/−4
- test/HTTP2/ServerSpec.hs +79/−1
ChangeLog.md view
@@ -1,153 +1,98 @@ # ChangeLog for http2 +## 5.4.8++* Reset a stream cancelled with an asynchronous exception instead of+ closing the connection.+ [#214](https://github.com/kazu-yamamoto/http2/pull/214)+* Server: tickle the worker's timer on sending, so that a response+ streamed longer than the timeout is not killed.+ [#174](https://github.com/kazu-yamamoto/http2/pull/174)+* `freeSimpleConfig` no longer calls the no-op `killManager`.+ Timeout actions registered with `confTimeoutManager` must be+ cancelled by their owner.+ [#215](https://github.com/kazu-yamamoto/http2/pull/215)+ ## 5.4.7 -* A valid request could close the whole connection, with every other- stream on it:- - a Huffman-coded field value longer than 4096 octets, such as a long- token in `authorization`, was taken for a truncated block- [#201](https://github.com/kazu-yamamoto/http2/pull/201);- - so was a header block with no fields, which is what empty trailers- are sent as [#202](https://github.com/kazu-yamamoto/http2/pull/202);- - a malformed field (an upper-case name, a pseudo-header out of place,- more than 200 fields) left the rest of its block undecoded, and the- HPACK tables out of step. The block is now decoded to the end and the- message refused with RST_STREAM(PROTOCOL_ERROR) on its stream alone- (RFC 9113, section 8.1.1).- [#211](https://github.com/kazu-yamamoto/http2/pull/211)-* Flow control lost octets, so that a long-lived connection could stall:- - the padding of DATA frames was charged to both windows and never- given back [#204](https://github.com/kazu-yamamoto/http2/pull/204);- - DATA refused on a stream in the wrong state, or ignored on a stream we- had reset, was not charged to the connection window, though the peer- had charged it [#205](https://github.com/kazu-yamamoto/http2/pull/205);- - on a client, the rest of a response that `processResponse` did not- read to the end, or that it threw on, was never given back, and its- stream held a slot of the server's SETTINGS_MAX_CONCURRENT_STREAMS.- Such a stream is now reset with CANCEL.- [#207](https://github.com/kazu-yamamoto/http2/pull/207)- - on a client, the DATA of a push nobody asked for was held against the- connection window for good. Pushes now give it back as it arrives.- [#208](https://github.com/kazu-yamamoto/http2/pull/208)-* A padded body that matched its content-length was reset as malformed:- the padding was counted into its length.+* Decode a Huffman-coded field value longer than 4096 octets.+ [#201](https://github.com/kazu-yamamoto/http2/pull/201)+* Accept a header block with no fields, as empty trailers are sent.+ [#202](https://github.com/kazu-yamamoto/http2/pull/202)+* Decode a malformed field block to the end and reset only its stream.+ [#211](https://github.com/kazu-yamamoto/http2/pull/211)+* Give the padding of DATA frames back to the flow control windows.+ [#204](https://github.com/kazu-yamamoto/http2/pull/204)+* Charge refused or ignored DATA to the connection window.+ [#205](https://github.com/kazu-yamamoto/http2/pull/205)+* Client: reset with CANCEL a response not read to the end.+ [#207](https://github.com/kazu-yamamoto/http2/pull/207)+* Client: give back the window used by unwanted pushes.+ [#208](https://github.com/kazu-yamamoto/http2/pull/208)+* Do not count padding into the length checked against content-length. [#203](https://github.com/kazu-yamamoto/http2/pull/203)-* A response carrying a push waited for ever when the client announced- SETTINGS_MAX_CONCURRENT_STREAMS of 0, the way to refuse pushes. A push- there is no room for is now not made.+* Make no push when the client allows no concurrent streams. [#206](https://github.com/kazu-yamamoto/http2/pull/206)-* GOAWAY:- - the last stream identifier of the server's GOAWAY left out streams- whose handlers were still running, so a client could send again a- request that had been acted on- [#209](https://github.com/kazu-yamamoto/http2/pull/209);- - a GOAWAY with NO_ERROR closed the connection at once, failing every- stream in flight. Streams up to its last stream identifier now go- on, those above it fail with `ConnectionIsClosed`, no new stream is- opened, and the connection closes once nothing is left; on a client,- the client function is let finish.- [#210](https://github.com/kazu-yamamoto/http2/pull/210)-* With the connection window shut, nothing went out at all, though only- DATA is flow-controlled: not the response to a request with no body,- not RST_STREAM. DATA now waits for the window on its own.+* Include running streams in the last stream identifier of GOAWAY.+ [#209](https://github.com/kazu-yamamoto/http2/pull/209)+* Let streams up to the last stream identifier finish after a GOAWAY+ with NO_ERROR.+ [#210](https://github.com/kazu-yamamoto/http2/pull/210)+* Send frames other than DATA while the connection window is shut. [#212](https://github.com/kazu-yamamoto/http2/pull/212) ## 5.4.6 -* Security: a regression in 5.4.5. Since stream errors reset the stream- rather than the connection, a peer could have the server reset streams- for it -- with a PRIORITY on a stream depending on itself, DATA on a- half-closed stream, and the like -- and so free concurrency slots while- the handlers went on running, without ever sending RST_STREAM itself- (MadeYouReset, CVE-2025-8671). Resets we send because of the peer now- count against `rstRateLimit` with the peer's own.+* Security: count the resets we send against `rstRateLimit`+ (MadeYouReset, CVE-2025-8671). [#190](https://github.com/kazu-yamamoto/http2/pull/190)-* Security: a PRIORITY frame for a stream that was never opened created- the stream and took a concurrency slot for good, so 64 PRIORITY frames- were enough to have every later request refused.+* Security: do not create a stream for a PRIORITY frame. [#195](https://github.com/kazu-yamamoto/http2/pull/195)-* Security: a SETTINGS_INITIAL_WINDOW_SIZE that overflowed a stream's- window stopped the sender without a word, leaving the connection open- and silent. It is now a connection error of type FLOW_CONTROL_ERROR,- and any failure of the sender closes the connection.+* Security: treat an overflowing SETTINGS_INITIAL_WINDOW_SIZE as+ FLOW_CONTROL_ERROR. [#196](https://github.com/kazu-yamamoto/http2/pull/196)-* The HPACK dynamic table lost entries, or had the encoder send the wrong- one (index 61 of the static table), once it held as many entries as it- has room for -- which a small or odd SETTINGS_HEADER_TABLE_SIZE from the- peer makes easy. Headers were silently wrong on both sides.+* Fix the HPACK dynamic table when it is full. [#192](https://github.com/kazu-yamamoto/http2/pull/192)-* A Huffman-coded string of 16K or more was corrupted by the encoder: the- length's fourth octet overwrote the start of the code.+* Fix encoding a Huffman-coded string of 16K or more. [#188](https://github.com/kazu-yamamoto/http2/pull/188)-* Header blocks and trailers larger than a frame are sent and received as- HEADERS and CONTINUATION frames, and the header blocks of streams that- are already reset are still decoded, so that the HPACK tables stay in- step. Thanks to Edsko de Vries.+* Send and receive large header blocks with CONTINUATION frames, and keep+ decoding the header blocks of reset streams. Thanks to Edsko de Vries. [#187](https://github.com/kazu-yamamoto/http2/pull/187) [#189](https://github.com/kazu-yamamoto/http2/pull/189)-* A race between the receiver and the sender lost a stream's half-closed- state, so that it was never removed from the stream table: with both- ends streaming, a client ran out of streams and a server refused every- new one.+* Fix a race that lost a stream's half-closed state. [#193](https://github.com/kazu-yamamoto/http2/pull/193)-* A client no longer rejects a response that has no content but a- non-zero content-length, as responses to HEAD and 304 responses do.+* Client: accept a response with no content but a non-zero+ content-length. [#194](https://github.com/kazu-yamamoto/http2/pull/194)-* A client request that failed before it was queued -- a `requestFile` for- a file that cannot be opened, say -- made every later request on the- connection wait for ever.+* Client: a request that fails before it is queued no longer blocks later+ ones. [#198](https://github.com/kazu-yamamoto/http2/pull/198)-* Server push: a PUSH_PROMISE could come after the response it belongs- to, and pushed streams were never closed, so a connection stopped after- 64 pushes.+* Server push: send PUSH_PROMISE before the response and close pushed+ streams. [#199](https://github.com/kazu-yamamoto/http2/pull/199)-* An upload through `runIO` larger than the stream's window was cut short- with END_STREAM after the first window's worth.+* Fix an upload through `runIO` larger than the stream's window. [#200](https://github.com/kazu-yamamoto/http2/pull/200)-* GHC 9.12 and later, with `-O`, miscompile a value holding a- never-returning streaming body into one with no body- ([GHC #27857](https://gitlab.haskell.org/ghc/ghc/-/work_items/27857)).- The test suite works around it.+* Tests: work around a GHC 9.12 miscompilation. [#197](https://github.com/kazu-yamamoto/http2/pull/197) ## 5.4.5 -* Security: frame payload decoders read their fixed-size fields without- checking that the payload holds them, so a truncated frame, or padding- covering a field, read past the end of the buffer -- and an empty payload- is the shared empty `ByteString`, whose pointer is null. An- unauthenticated peer could segfault the process with 33 bytes.+* Security: check the length of a frame payload before decoding its+ fixed-size fields. [#182](https://github.com/kazu-yamamoto/http2/pull/182)-* Security: HPACK integer decoding overflowed `Int` silently, so a long- enough encoding decoded to whatever value the sender aimed at and two- different byte strings could decode to the same header. Integers are now- bounded and over-long encodings are a decoding error, as RFC 7541- section 5.1 requires.+* Security: bound HPACK integer decoding. [#181](https://github.com/kazu-yamamoto/http2/pull/181)-* A RST_STREAM gave a stream's concurrency slot back twice, so a peer could- walk `SETTINGS_MAX_CONCURRENT_STREAMS` upwards and hold open as many- streams as it liked.+* Give a stream's concurrency slot back only once on RST_STREAM. [#178](https://github.com/kazu-yamamoto/http2/pull/178)-* A stream reset while its response was still being produced left the- worker blocked until the timeout manager killed it, one thread per reset- stream.+* Stop the worker of a stream reset while its response is produced. [#179](https://github.com/kazu-yamamoto/http2/pull/179)-* Stream errors now reset the stream and the connection carries on, as- RFC 9113 section 5.4.2 requires. A field block abandoned part-way is- still a connection error, since the HPACK tables have diverged by then.+* Reset the stream, not the connection, on a stream error. [#183](https://github.com/kazu-yamamoto/http2/pull/183)-* A stream over `SETTINGS_MAX_CONCURRENT_STREAMS` is refused with- RST_STREAM(REFUSED_STREAM) rather than ending the connection.+* Refuse a stream over `SETTINGS_MAX_CONCURRENT_STREAMS` with+ RST_STREAM(REFUSED_STREAM). [#184](https://github.com/kazu-yamamoto/http2/pull/184)-* `DecodeError` has a new constructor, `TooLargeInteger`. Strictly this is- a breaking change -- an exhaustive match on `DecodeError` no longer- compiles -- but it ships as a patch version on purpose: no package on- Hackage names any constructor of that type, while a minor bump would- shut out every dependant carrying a `< 5.5` bound, these security fixes- along with it.-* A malformed request now reaches a client as `StreamResetIsReceived` on- the stream it concerns, where it used to arrive as- `ConnectionErrorIsReceived` on the connection.+* `DecodeError` has a new constructor, `TooLargeInteger`.+* A malformed request reaches a client as `StreamResetIsReceived`. ## 5.4.4
Network/HTTP2/Client/Run.hs view
@@ -289,7 +289,7 @@ else do (pop, out) <- makeOutput strm ot pushOutput sid out `E.onException` abandon sid- lc <- newLoopCheck strm mtbq+ lc <- newLoopCheck strm mtbq Nothing T.forkManaged threadManager label $ syncWithSender' ctx pop lc where label = "H2 request sender for stream " ++ show (streamNumber strm)
Network/HTTP2/H2/Config.hs view
@@ -37,7 +37,12 @@ return Config{..} -- | Deallocating the resource of the simple configuration.+--+-- This does not cancel timeout actions registered with+-- 'confTimeoutManager'. Since time-manager 0.3, a manager holds no+-- registrations, so there is nothing to kill: an action registered with+-- 'System.TimeManager.register' runs even after this returns unless it+-- is cancelled with 'System.TimeManager.cancel'. Use+-- 'System.TimeManager.withHandle', which cancels it when the scope ends. freeSimpleConfig :: Config -> IO ()-freeSimpleConfig conf = do- free $ confWriteBuffer conf- T.killManager $ confTimeoutManager conf+freeSimpleConfig conf = free $ confWriteBuffer conf
Network/HTTP2/H2/Sender.hs view
@@ -253,17 +253,23 @@ return off' ----------------------------------------------------------------- handler strm off e = do- resetStream strm InternalError e- return off-- resetStream :: Stream -> ErrorCode -> E.SomeException -> IO ()- resetStream strm err e+ handler strm off e | isAsyncException e = E.throwIO e | otherwise = do- closed ctx strm (ResetByMe e)- let rst = resetFrame err $ streamNumber strm- enqueueControl controlQ $ CFrames Nothing [rst]+ resetStream strm InternalError e+ return off++ -- 'e' is the reason for the reset, not something thrown to the+ -- sender. It may well be asynchronous: a streaming body killed+ -- by 'cancel' passes the exception to 'outBodyCancel'. Throwing+ -- it here would kill the sender and the whole connection with it.+ -- Asynchronous exceptions actually thrown to the sender are+ -- re-thrown by 'handler'.+ resetStream :: Stream -> ErrorCode -> E.SomeException -> IO ()+ resetStream strm err e = do+ closed ctx strm (ResetByMe e)+ let rst = resetFrame err $ streamNumber strm+ enqueueControl controlQ $ CFrames Nothing [rst] resetStreamWith :: Stream -> Maybe E.SomeException -> IO () resetStreamWith strm (Just err) =
Network/HTTP2/H2/Sync.hs view
@@ -108,13 +108,15 @@ case s of Done -> return () Cont newout -> do+ mapM_ T.tickle (lcTimeHandle lc) cont <- checkLoop lc when cont $ do enqueueOutput outputQ newout loop -newLoopCheck :: Stream -> Maybe (TBQueue StreamingChunk) -> IO LoopCheck-newLoopCheck strm mtbq = do+newLoopCheck+ :: Stream -> Maybe (TBQueue StreamingChunk) -> Maybe T.Handle -> IO LoopCheck+newLoopCheck strm mtbq mth = do tovar <- newTVarIO False return $ LoopCheck@@ -122,6 +124,7 @@ , lcTBQ = mtbq , lcTimeout = tovar , lcWindow = streamTxFlow strm+ , lcTimeHandle = mth } data LoopCheck = LoopCheck@@ -129,6 +132,7 @@ , lcTBQ :: Maybe (TBQueue StreamingChunk) , lcTimeout :: TVar Bool , lcWindow :: TVar TxFlow+ , lcTimeHandle :: Maybe T.Handle } checkLoop :: LoopCheck -> IO Bool
Network/HTTP2/Server/Worker.hs view
@@ -41,7 +41,7 @@ #endif } request = Request req'- lc <- newLoopCheck strm Nothing+ lc <- newLoopCheck strm Nothing (Just th) server request aux $ sendResponse conf ctx lc strm request adjustRxWindow ctx strm where@@ -65,7 +65,7 @@ -- ordering with respect to the final response. sendInformational :: Context -> Stream -> Status -> ResponseHeaders -> IO () sendInformational ctx strm st hdrs = do- lc <- newLoopCheck strm Nothing+ lc <- newLoopCheck strm Nothing Nothing let hdr = (":status", C8.pack (show (statusCode st))) : hdrs syncWithSender ctx strm (OInformational hdr) lc #endif@@ -159,7 +159,7 @@ , (tokenPath, path) ] ot = OPush promiseRequest pid- lc <- newLoopCheck newstrm Nothing+ lc <- newLoopCheck newstrm Nothing Nothing syncWithSender ctx newstrm ot lc -- Reserved (local) until now. The peer sends nothing on a pushed -- stream, so its side is closed from here (RFC 9113, section 5.1:
http2.cabal view
@@ -1,6 +1,6 @@ cabal-version: 2.0 name: http2-version: 5.4.7+version: 5.4.8 license: BSD3 license-file: LICENSE maintainer: Kazu Yamamoto <kazu@iij.ad.jp>
test/HTTP2/FrameSpec.hs view
@@ -32,14 +32,14 @@ -- Six octets is the smallest payload the header check accepts for -- PADDED and PRIORITY together, and a Pad Length of five leaves -- none of the five priority octets behind.- let flags = setPadded $ setPriority defaultFlags- header = FrameHeader 6 flags 1+ let flags' = setPadded $ setPriority defaultFlags+ header = FrameHeader 6 flags' 1 decodeError FrameHeaders header (BS.pack [5, 0, 0, 0, 0, 0]) `shouldBe` Just FrameSizeError it "rejects a padded PUSH_PROMISE whose padding covers the promised id" $ do- let flags = setPadded defaultFlags- header = FrameHeader 5 flags 1+ let flags' = setPadded defaultFlags+ header = FrameHeader 5 flags' 1 decodeError FramePushPromise header (BS.pack [4, 0, 0, 0, 0]) `shouldBe` Just FrameSizeError
test/HTTP2/ServerSpec.hs view
@@ -147,6 +147,21 @@ threadDelay 10000 runStreamErrorClient + it "resets a stream cancelled with an asynchronous exception and goes on" $+ -- The exception is only the reason for the reset. The sender+ -- once threw it, which took the whole connection down.+ E.bracket (forkIO runServer) killThread $ \_ -> do+ threadDelay 10000+ runAsyncCancelClient++ it "keeps a stream that goes on sending past the timeout" $+ -- The worker's timer was only tickled by reading the request+ -- body, so a response streamed for longer than the timeout was+ -- killed half-way (#173).+ E.bracket (forkIO runServerShortTimeout) killThread $ \_ -> do+ threadDelay 10000+ runDripClient `shouldReturn` dripChunks+ it "limits the resets a peer can make us send (MadeYouReset)" $ E.bracket (forkIO runServer) killThread $ \_ -> do threadDelay 10000@@ -550,6 +565,21 @@ freeSimpleConfig (\conf -> run sconf conf server) +-- | A server whose timeout is one second. Like Warp, it sets+-- 'confReadNTimeout', so the receiver has no timer of its own, which would+-- close a connection the client sends nothing on.+runServerShortTimeout :: IO ()+runServerShortTimeout = runTCPServer (Just host) port runHTTP2Server+ where+ alloc s = do+ conf <- allocSimpleConfig' s 32768 1000000+ return conf{confReadNTimeout = True}+ runHTTP2Server s =+ E.bracket+ (alloc s)+ freeSimpleConfig+ (\conf -> run defaultServerConfig conf server)+ runServerMaxConc1 :: IO () runServerMaxConc1 = runTCPServer (Just host) port runHTTP2Server where@@ -575,7 +605,7 @@ (\conf -> run defaultServerConfig conf cancelServer) cancelServer _req _aux sendResponse = do threadDelay 200000- sendResponse responseHello []+ _ <- sendResponse responseHello [] putMVar doneVar () runFakeServer :: MVar ByteString -> IO ()@@ -694,6 +724,10 @@ [("link", "</app.js>; rel=preload; as=script")] sendResponse responseHello [] Just "/stream" -> sendResponse responseInfinite []+ -- Some of the body, then cancelled with an asynchronous exception.+ Just "/cancel-async" -> sendResponse responseCancelAsync []+ -- Longer than the timeout of 'runServerShortTimeout', a chunk at a time.+ Just "/drip" -> sendResponse responseDrip [] -- Like /stream, but going quietly once the client resets it. Just "/endless" -> sendResponse responseEndless [] Just "/not-modified" -> sendResponse (responseNoBody notModified304 bigLength) []@@ -792,6 +826,22 @@ quiet :: E.SomeException -> IO () quiet _ = return () +dripChunks :: Int+dripChunks = 8++-- | A chunk every 300 milliseconds: 2.4 seconds in all.+responseDrip :: Response+responseDrip = responseStreaming ok200 [] $ \write flush ->+ replicateM_ dripChunks $ do+ write (byteString "x") >> flush+ threadDelay 300000++responseCancelAsync :: Response+responseCancelAsync = responseStreamingIface ok200 [] $ \iface -> do+ outBodyPush iface "x"+ outBodyFlush iface+ outBodyCancel iface $ Just $ E.toException AsyncCancelled+ responseInfinite :: Response responseInfinite = responseStreaming ok200 header body where@@ -1670,6 +1720,34 @@ -- "connection" it is not one of the headers the sender strips. let bad = C.requestNoBody methodGet "/" [("te", "gzip")] sendRequest bad (\_ -> return ()) `shouldThrow` streamWasReset+ let good = C.requestNoBody methodGet "/" []+ sendRequest good $ \rsp ->+ C.responseStatus rsp `shouldBe` Just ok200+ where+ cliconf = C.defaultClientConfig{C.authority = host}++-- | How many octets of '/drip' arrive.+runDripClient :: IO Int+runDripClient = runTCPClient host port $ \s ->+ E.bracket (allocSimpleConfig s 4096) freeSimpleConfig $ \conf ->+ C.run cliconf conf $ \sendRequest _aux -> do+ let req = C.requestNoBody methodGet "/drip" []+ count rsp n = do+ bs <- C.getResponseBodyChunk rsp+ if B.null bs then return n else count rsp (n + B.length bs)+ sendRequest req $ \rsp -> count rsp 0+ where+ cliconf = C.defaultClientConfig{C.authority = host}++runAsyncCancelClient :: IO ()+runAsyncCancelClient = runTCPClient host port $ \s ->+ E.bracket (allocSimpleConfig s 4096) freeSimpleConfig $ \conf ->+ C.run cliconf conf $ \sendRequest _aux -> do+ let req = C.requestNoBody methodGet "/cancel-async" []+ drain rsp = do+ bs <- C.getResponseBodyChunk rsp+ unless (B.null bs) $ drain rsp+ sendRequest req drain `shouldThrow` streamWasReset let good = C.requestNoBody methodGet "/" [] sendRequest good $ \rsp -> C.responseStatus rsp `shouldBe` Just ok200