fix(http2): check content-length, fix sending RST
Fixes #35209 Closes #35378
Time spent: 60h
[!CAUTION] These bug fixes are potentially
semver-major
!
- Fixed
stream.close(NGHTTP2_CANCEL)
closing with0
akaNGHTTP2_NO_ERROR
. -
serverStream.destroy()
closed with0
, nowNGHTTP2_INTERNAL_ERROR
. -
clientStream.destroy()
closed with0
, nowNGHTTP2_CANCEL
. - Mismatched
content-length
now throwsNGHTTP2_PROTOCOL_ERROR
as according to the spec. - Fixed
GOAWAY
(server -> client) being greeted withGOAWAY
(client -> server) as it's against the spec. - Fixed streams being always closed with
NGHTTP2_CANCEL
on socket close, now respectsgoawayCode
anddestroyCode
. For client, the default remainsNGHTTP2_CANCEL
. For server, the default now isNGHTTP2_INTERNAL_ERROR
. - Fixed
stream.rstCode
being 0 while active - docs say it should be undefined. - Fixed preferring
sessionCode
overrstCode
when destroying a stream with an error. - Fixed streams being closed with
NO_ERROR
if session receivedGOAWAY
withNO_ERROR
and remote closed connection. - Fixed streams being closed with
NO_ERROR
if session sentGOAWAY
withNO_ERROR
and destroyed. - Fixed
GOAWAY
preventingRST_STREAM
from being sent beforeGOAWAY
. nghttp2 correctly assumes that it should preventRST_STREAM
from being sent but incorretly applies it to all frames in a packet instead of frames defined afterGOAWAY
. This is not the only thing that nghttp2 does wrong: - Fixed
benchmark/http2/headers.js
callingclient.destroy()
(resulting in dropped requests). Now callsclient.close()
which closes gracefully. -
connectStreamSocket.destroy()
now closes withNGHTTP2_CONNECT_ERROR
. - Fixed double
GOAWAY
onsession.close()
. - Fixed queued RST_STREAM and GOAWAY being dropped if
Http2Session::Close()
and previous writes weren't finished yet. - Fixed stream frame errors closing the entire session instead of just the stream.
- Fixed stream frame errors sending
RST_STREAM
on idle streams (to reproduce 16. needs to be fixed).
Sorry so many bugs are fixed in a single PR but it's impossible to fix one without fixing them all!
Best if https://github.com/nghttp2/nghttp2/pull/1508 got merged before this, but it has been open for years nghttp2_session.c
/cc @jasnell