http2: send GOAWAY properly & don't continue reading unnecessarily by apapirovski · Pull Request #20772 · nodejs/node

@apapirovski added the http2

Issues or PRs related to the http2 subsystem.

label

May 16, 2018

@apapirovski apapirovski changed the title http2: don't prematurely destroy the socket http2: send GOAWAY properly & don't continue reading unnecessarily

May 19, 2018

This was referenced

May 19, 2018

@apapirovski

Currently http2 does not properly submit Goaway frames when
a session is being destroyed. It also doesn't properly
handle when the other party severs the connection after
sending a Goaway frame, even though it should.

kjin pushed a commit to kjin/node that referenced this pull request

Aug 23, 2018
Currently http2 does not properly submit GOAWAY frames when a session
is being destroyed. It also doesn't properly handle when the other
party severs the connection after sending a GOAWAY frame, even though
it should.

Edge, IE & Safari are currently unable to handle empty TRAILERS
frames despite them being correctly to spec. Instead send an empty
DATA frame with END_STREAM flag in those situations.

Fix and adjust several flaky and/or incorrect tests.

PR-URL: nodejs#20772
Fixes: nodejs#20705
Fixes: nodejs#20750
Fixes: nodejs#20850
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

kjin pushed a commit to kjin/node that referenced this pull request

Aug 23, 2018
It's possible for the connections to take too long and since the server
is already unrefed, the process will just exit. Instead adjust the test
so that server unref only happens after all sessions have been
successfuly established and unrefed. That still tests the same condition
but will not fail under load.

PR-URL: nodejs#20772
Fixes: nodejs#20705
Fixes: nodejs#20750
Fixes: nodejs#20850
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

@kjin kjin mentioned this pull request

Sep 19, 2018

3 tasks

kjin pushed a commit to kjin/node that referenced this pull request

Sep 25, 2018
Currently http2 does not properly submit GOAWAY frames when a session
is being destroyed. It also doesn't properly handle when the other
party severs the connection after sending a GOAWAY frame, even though
it should.

Edge, IE & Safari are currently unable to handle empty TRAILERS
frames despite them being correctly to spec. Instead send an empty
DATA frame with END_STREAM flag in those situations.

Fix and adjust several flaky and/or incorrect tests.

PR-URL: nodejs#20772
Fixes: nodejs#20705
Fixes: nodejs#20750
Fixes: nodejs#20850
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

kjin pushed a commit to kjin/node that referenced this pull request

Sep 25, 2018
It's possible for the connections to take too long and since the server
is already unrefed, the process will just exit. Instead adjust the test
so that server unref only happens after all sessions have been
successfuly established and unrefed. That still tests the same condition
but will not fail under load.

PR-URL: nodejs#20772
Fixes: nodejs#20705
Fixes: nodejs#20750
Fixes: nodejs#20850
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

kjin pushed a commit to kjin/node that referenced this pull request

Oct 16, 2018
Currently http2 does not properly submit GOAWAY frames when a session
is being destroyed. It also doesn't properly handle when the other
party severs the connection after sending a GOAWAY frame, even though
it should.

Edge, IE & Safari are currently unable to handle empty TRAILERS
frames despite them being correctly to spec. Instead send an empty
DATA frame with END_STREAM flag in those situations.

Fix and adjust several flaky and/or incorrect tests.

PR-URL: nodejs#20772
Fixes: nodejs#20705
Fixes: nodejs#20750
Fixes: nodejs#20850
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

kjin pushed a commit to kjin/node that referenced this pull request

Oct 16, 2018
It's possible for the connections to take too long and since the server
is already unrefed, the process will just exit. Instead adjust the test
so that server unref only happens after all sessions have been
successfuly established and unrefed. That still tests the same condition
but will not fail under load.

PR-URL: nodejs#20772
Fixes: nodejs#20705
Fixes: nodejs#20750
Fixes: nodejs#20850
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

BethGriggs pushed a commit that referenced this pull request

Oct 17, 2018
Currently http2 does not properly submit GOAWAY frames when a session
is being destroyed. It also doesn't properly handle when the other
party severs the connection after sending a GOAWAY frame, even though
it should.

Edge, IE & Safari are currently unable to handle empty TRAILERS
frames despite them being correctly to spec. Instead send an empty
DATA frame with END_STREAM flag in those situations.

Fix and adjust several flaky and/or incorrect tests.

Backport-PR-URL: #22850
PR-URL: #20772
Fixes: #20705
Fixes: #20750
Fixes: #20850
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

BethGriggs pushed a commit that referenced this pull request

Oct 17, 2018
It's possible for the connections to take too long and since the server
is already unrefed, the process will just exit. Instead adjust the test
so that server unref only happens after all sessions have been
successfuly established and unrefed. That still tests the same condition
but will not fail under load.

Backport-PR-URL: #22850
PR-URL: #20772
Fixes: #20705
Fixes: #20750
Fixes: #20850
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>