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.

Sign up for GitHub

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

Jump to bottom

HTTP2: Calling res.end after res.writeHead(204) cause an error #21740

Closed
RidgeA opened this issue Jul 10, 2018 · 2 comments
Closed

HTTP2: Calling res.end after res.writeHead(204) cause an error #21740

RidgeA opened this issue Jul 10, 2018 · 2 comments
Labels
http2 Issues or PRs related to the http2 subsystem.

Comments

@RidgeA
Copy link
Contributor

RidgeA commented Jul 10, 2018

  • Version: v10.6.0
  • Platform: Linux localhost.localdomain 4.17.3-200.fc28.x86_64 deps: update openssl to 1.0.1j #1 SMP Tue Jun 26 14:17:07 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux (Fedora 28)
  • Subsystem: http2

Hi!

I'm playing around with http2 module and server push and faced with a strange error.

Invoking of res.end() method after res.writeHeader(204) causes error:

internal/http2/compat.js:377
  this.sendTrailers(this[kResponse][kTrailers]);
                                   ^

TypeError: Cannot read property 'Symbol(trailers)' of undefined
    at ServerHttp2Stream.onStreamTrailersReady (internal/http2/compat.js:377:36)
    at ServerHttp2Stream.emit (events.js:182:13)
    at Http2Stream.onStreamTrailers [as ontrailers] (internal/http2/core.js:311:15)

Code to reproduce:

"use strict";

const http2 = require('http2');
const fs = require('fs');
const cert = {
  key: fs.readFileSync('./cert/key.pem'),
  cert: fs.readFileSync('./cert/cert.pem')
};
const html =
`
<!doctype html>
<head>
    <title>HTTP/2 Test</title>
    <link rel="stylesheet" href="/styles.css" type="text/css">
    <script src="/bundle.js" async></script>
</head>
<body>
<h1>Hello</h1>
</body>
</html>
`;

http2.createSecureServer(cert, (req, res) => {

  const {url} = req;

  if (['/index.html', '/'].includes(url)) {
    res.writeHead(200, {'content-type':'text/html'});
    res.end(html);
  } else {
    res.writeHead(204);
    res.end();
  }

}).listen(8443, err => {
  if (!err) {
    console.log(`Server started at ${8443}.`)
  }
});

After digging into the issue, I found out that if we set 204, 205 and 304 status codes the stream are closed automatically - see https://github.com/nodejs/node/blob/master/lib/internal/http2/core.js#L2266 and https://github.com/nodejs/node/blob/master/lib/internal/http2/core.js#L2281

Calling of req.end method after that causes an error (see above).

According to the documentation req.end() MUST be called for each request: https://nodejs.org/api/http2.html#http2_response_end_data_encoding_callback
but it is impossible due to closing the stream caused by calling writeHeader method.

I believe, in order to keep backward compatibility with http/https modules, the res.end method shouldn't throw an error in such case.
@RidgeA RidgeA changed the title Http2: Calling res.end after res.writeHead(204) cause an error. HTTP2: Calling res.end after res.writeHead(204) cause an error. Jul 10, 2018
@RidgeA RidgeA changed the title HTTP2: Calling res.end after res.writeHead(204) cause an error. HTTP2: Calling res.end after res.writeHead(204) cause an error Jul 10, 2018
@addaleax addaleax added the http2 Issues or PRs related to the http2 subsystem. label Jul 10, 2018
@addaleax
Copy link
Member

@nodejs/http2

@RidgeA RidgeA mentioned this issue Jul 11, 2018
Closed
3 tasks
@RidgeA
Copy link
Contributor Author

RidgeA commented Jul 11, 2018

PR to fix this issue - #21764

targos pushed a commit that referenced this issue Jul 16, 2018
@RidgeA @targos
http2: remove waitTrailers listener after closing a stream
238ef58 
When `writeHeader` of `Http2ServerResponse` instance are called with
204, 205 and 304 status codes an underlying stream closes.
If call `end` method after sending any of these status codes it will
cause an error `TypeError: Cannot read property 'Symbol(trailers)' of
undefined` because a reference to `Http2ServerResponse` instance
associated with Http2Stream already was deleted.
The closing of stream causes emitting `waitTrailers` event and, when
this event handles inside `onStreamTrailerReady` handler, there is
no reference to Http2ServerResponse instance.

Fixes: #21740
PR-URL: #21764
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
kjin pushed a commit to kjin/node that referenced this issue Aug 23, 2018
@RidgeA @kjin
http2: remove waitTrailers listener after closing a stream
98e81c0 
When `writeHeader` of `Http2ServerResponse` instance are called with
204, 205 and 304 status codes an underlying stream closes.
If call `end` method after sending any of these status codes it will
cause an error `TypeError: Cannot read property 'Symbol(trailers)' of
undefined` because a reference to `Http2ServerResponse` instance
associated with Http2Stream already was deleted.
The closing of stream causes emitting `waitTrailers` event and, when
this event handles inside `onStreamTrailerReady` handler, there is
no reference to Http2ServerResponse instance.

Fixes: nodejs#21740
PR-URL: nodejs#21764
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
kjin pushed a commit to kjin/node that referenced this issue Sep 25, 2018
@RidgeA @kjin
http2: remove waitTrailers listener after closing a stream
1a15129 
When `writeHeader` of `Http2ServerResponse` instance are called with
204, 205 and 304 status codes an underlying stream closes.
If call `end` method after sending any of these status codes it will
cause an error `TypeError: Cannot read property 'Symbol(trailers)' of
undefined` because a reference to `Http2ServerResponse` instance
associated with Http2Stream already was deleted.
The closing of stream causes emitting `waitTrailers` event and, when
this event handles inside `onStreamTrailerReady` handler, there is
no reference to Http2ServerResponse instance.

Fixes: nodejs#21740
PR-URL: nodejs#21764
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
kjin pushed a commit to kjin/node that referenced this issue Oct 16, 2018
@RidgeA @kjin
http2: remove waitTrailers listener after closing a stream
9a0d117 
When `writeHeader` of `Http2ServerResponse` instance are called with
204, 205 and 304 status codes an underlying stream closes.
If call `end` method after sending any of these status codes it will
cause an error `TypeError: Cannot read property 'Symbol(trailers)' of
undefined` because a reference to `Http2ServerResponse` instance
associated with Http2Stream already was deleted.
The closing of stream causes emitting `waitTrailers` event and, when
this event handles inside `onStreamTrailerReady` handler, there is
no reference to Http2ServerResponse instance.

Fixes: nodejs#21740
PR-URL: nodejs#21764
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
BethGriggs pushed a commit that referenced this issue Oct 17, 2018
@RidgeA @BethGriggs
http2: remove waitTrailers listener after closing a stream
e5175e6 
When `writeHeader` of `Http2ServerResponse` instance are called with
204, 205 and 304 status codes an underlying stream closes.
If call `end` method after sending any of these status codes it will
cause an error `TypeError: Cannot read property 'Symbol(trailers)' of
undefined` because a reference to `Http2ServerResponse` instance
associated with Http2Stream already was deleted.
The closing of stream causes emitting `waitTrailers` event and, when
this event handles inside `onStreamTrailerReady` handler, there is
no reference to Http2ServerResponse instance.

Fixes: #21740
Backport-PR-URL: #22850
PR-URL: #21764
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
@aliceUnhinged613 aliceUnhinged613 mentioned this issue May 16, 2022
Open
This was referenced May 16, 2022
Open
Open
@kissofire kissofire mentioned this issue May 16, 2022
Open
@wedataintelligence wedataintelligence mentioned this issue May 16, 2022
Open
@ron-a20 ron-a20 mentioned this issue May 16, 2022
Open
@molllyn1 molllyn1 mentioned this issue May 17, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@addaleax @RidgeA