From 5ab32551d50af8aa702ce272a02b4e84f12d57a7 Mon Sep 17 00:00:00 2001
From: XadillaX <i@2333.moe>
Date: Fri, 9 Feb 2018 19:08:13 +0800
Subject: [PATCH 1/2] http2: fix endless loop when write an empty string

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: https://github.com/nodejs/node/issues/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
---
 lib/internal/http2/core.js | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index 5f811ca8af403a..9855eafe1e2163 100644
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -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];
@@ -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];

From 83fb36a3288b59e2f8edf832f1eb411615209df0 Mon Sep 17 00:00:00 2001
From: XadillaX <i@2333.moe>
Date: Sun, 11 Feb 2018 15:57:35 +0800
Subject: [PATCH 2/2] add test cases

---
 .../test-http2-client-write-empty-string.js   | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 test/parallel/test-http2-client-write-empty-string.js

diff --git a/test/parallel/test-http2-client-write-empty-string.js b/test/parallel/test-http2-client-write-empty-string.js
new file mode 100644
index 00000000000000..757dde906cbfc8
--- /dev/null
+++ b/test/parallel/test-http2-client-write-empty-string.js
@@ -0,0 +1,48 @@
+'use strict';
+
+const assert = require('assert');
+const http2 = require('http2');
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const server = http2.createServer();
+server.on('stream', common.mustCall((stream, headers, flags) => {
+  stream.respond({ 'content-type': 'text/html' });
+
+  let data = '';
+  stream.on('data', common.mustNotCall((chunk) => {
+    data += chunk.toString();
+  }));
+  stream.on('end', common.mustCall(() => {
+    stream.end(`"${data}"`);
+  }));
+}));
+
+server.listen(0, common.mustCall(() => {
+  const port = server.address().port;
+  const client = http2.connect(`http://localhost:${port}`);
+
+  const req = client.request({
+    ':method': 'POST',
+    ':path': '/'
+  });
+
+  req.on('response', common.mustCall((headers) => {
+    assert.strictEqual(headers[':status'], 200);
+    assert.strictEqual(headers['content-type'], 'text/html');
+  }));
+
+  let data = '';
+  req.setEncoding('utf8');
+  req.on('data', common.mustCallAtLeast((d) => data += d));
+  req.on('end', common.mustCall(() => {
+    assert.strictEqual(data, '""');
+    server.close();
+    client.close();
+  }));
+
+  req.write('');
+  req.end();
+}));