Skip to content

Commit

Permalink
http2: fix endless loop when write an empty string
Browse files Browse the repository at this point in the history
If we do `req.write("")` (an empty string) on Http2Stream, the gather
logic will be messed up:

```
while ((src_length = nghttp2_session_mem_send(session_, &src)) > 0) {
  printf("write\n");
  DEBUG_HTTP2SESSION2(this, "nghttp2 has %d bytes to send", src_length);
  CopyDataIntoOutgoing(src, src_length);
}
```

The logic above is in function `Http2Session::SendPendingData` under
src/node_http2.cc. This `while` will be endless when an empty string is
inside because `src_length` will always be 9.

And then the main event loop thread will be blocked and the memory used
will be larger and larger.

This pull request is to ignore empty string or buffer in `_write()` and
`_writev()` of Http2Stream.

Fixes: nodejs#18169
Refs: https://github.com/nodejs/node/blob/v9.5.0/src/node_http2.cc#L1481-L1484
Refs: https://github.com/nodejs/node/blob/v9.5.0/lib/_http_outgoing.js#L659-L661
  • Loading branch information
XadillaX committed Feb 11, 2018
1 parent 95a35bc commit 5ab3255
Showing 1 changed file with 10 additions and 0 deletions.
10 changes: 10 additions & 0 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -1622,6 +1622,11 @@ class Http2Stream extends Duplex {
if (!this.headersSent)
this[kProceed]();

if (!data.length) {
cb();
return;
}

const handle = this[kHandle];
const req = new WriteWrap();
req.stream = this[kID];
Expand Down Expand Up @@ -1659,6 +1664,11 @@ class Http2Stream extends Duplex {
if (!this.headersSent)
this[kProceed]();

if (!data.length) {
cb();
return;
}

const handle = this[kHandle];
const req = new WriteWrap();
req.stream = this[kID];
Expand Down

0 comments on commit 5ab3255

Please sign in to comment.