Skip to content
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

http2: Edge and IE broken response #20850

Closed
webcarrot opened this issue May 20, 2018 · 3 comments
Closed

http2: Edge and IE broken response #20850

webcarrot opened this issue May 20, 2018 · 3 comments
Labels
confirmed-bug Issues with confirmed bugs. http2 Issues or PRs related to the http2 subsystem.

Comments

@webcarrot
Copy link

webcarrot commented May 20, 2018

After upgrade nodejs from v9.11.1 to v10.1.0 Edge and IE not handle properly response from server or response is somehow broken.

Simple server:

const http2 = require('http2');

const server = http2.createSecureServer({
        allowHTTP1: true,
        cert: "some cert",
        key: "some key"
}, (req, res) => {
   console.log("message");
   res.writeHead(200);
   res.end("ok");
   // or "hack" setTimeout(()=>res.end("ok"), 100); // to make Edge get valid response more offen
}).listen(443);

server.on("session", () => {
  console.log("session");
})
  • session is created
  • request come
  • browser not handle response

Edge sometimes display response, IE never.
On Edge bigger responses are truncated randomly.
I see same behavior in plain mode - via haproxy.

IE and Edge version:
sample

@apapirovski
Copy link
Member

Hi @webcarrot. Thanks for reporting this. I wonder if #20772 might fix this issue. There are some issues currently around closing streams and sessions properly.

@apapirovski apapirovski added http2 Issues or PRs related to the http2 subsystem. confirmed-bug Issues with confirmed bugs. labels May 20, 2018
@apapirovski
Copy link
Member

Looks like this is broken due to the new trailers implementation. I'm working on a fix and will include it in the PR mentioned above. Don't think we'll be able to add a test for this one though.

/cc @nodejs/http2

@apapirovski
Copy link
Member

Will be fixed by #20772

apapirovski added a commit that referenced this issue May 22, 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: #20772
Fixes: #20705
Fixes: #20750
Fixes: #20850
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue May 22, 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: #20772
Fixes: #20705
Fixes: #20750
Fixes: #20850
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue May 22, 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: #20772
Fixes: #20705
Fixes: #20750
Fixes: #20850
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue May 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: #20772
Fixes: #20705
Fixes: #20750
Fixes: #20850
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue May 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: #20772
Fixes: #20705
Fixes: #20750
Fixes: #20850
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
kjin pushed a commit to kjin/node that referenced this issue 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 <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
kjin pushed a commit to kjin/node that referenced this issue 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 <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
kjin pushed a commit to kjin/node that referenced this issue 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 <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
kjin pushed a commit to kjin/node that referenced this issue 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 <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
kjin pushed a commit to kjin/node that referenced this issue 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 <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
kjin pushed a commit to kjin/node that referenced this issue 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 <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
BethGriggs pushed a commit that referenced this issue 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 <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
BethGriggs pushed a commit that referenced this issue 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 <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants