http2: preserve original chunk type

Signed-off-by: Darshan Sen <raisinten@gmail.com>

Author: RaisinTen

doc/api/diagnostics_channel.md

@@ -1246,7 +1246,9 @@ Emitted when a stream is received on the client.
 ##### Event: `'http2.client.stream.bodyChunkSent'`
 
 * `stream` {ClientHttp2Stream}
-* `chunk` {Buffer}
+* `writev` {boolean}
+* `data` {Buffer | string | Array\<Buffer | {chunk: Buffer|string, encoding: string}>}
+* `encoding` {string}
 
 Emitted when a chunk of the client stream body is being sent.
 

lib/internal/http2/core.js

@@ -186,8 +186,6 @@ const { UV_EOF } = internalBinding('uv');
 const { StreamPipe } = internalBinding('stream_pipe');
 const { _connectionListener: httpConnectionListener } = http;
 
-const { Buffer } = require('buffer');
-
 const dc = require('diagnostics_channel');
 const onClientStreamCreatedChannel = dc.channel('http2.client.stream.created');
 const onClientStreamStartChannel = dc.channel('http2.client.stream.start');
@@ -2306,30 +2304,11 @@ class Http2Stream extends Duplex {
     trackWriteState(this, req.bytes);
 
     if (this.session[kType] === NGHTTP2_SESSION_CLIENT && onClientStreamBodyChunkSentChannel.hasSubscribers) {
-      let chunk;
-
-      if (ArrayIsArray(data)) {
-        const buffers = [];
-        for (let i = 0; i < data.length; ++i) {
-          if (typeof data[i] === 'object') {
-            if (typeof data[i].chunk === 'string') {
-              buffers.push(Buffer.from(data[i].chunk, data[i].encoding));
-            } else {
-              buffers.push(data[i].chunk);
-            }
-          }
-        }
-
-        chunk = Buffer.concat(buffers);
-      } else if (typeof data === 'string') {
-        chunk = Buffer.from(data);
-      } else {
-        chunk = data;
-      }
-
       onClientStreamBodyChunkSentChannel.publish({
         stream: this,
-        chunk,
+        writev,
+        data,
+        encoding,
       });
     }
   }

test/parallel/test-diagnostics-channel-http2-client-stream-body-multiple-buffers-and-strings.js

@@ -0,0 +1,73 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+// This test ensures that the built-in HTTP/2 diagnostics channels are reporting
+// the diagnostics messages for the 'http2.client.stream.bodyChunkSent' and
+// 'http2.client.stream.bodySent' channels when ClientHttp2Streams bodies are
+// being sent with multiple Buffers and strings.
+
+const assert = require('assert');
+const dc = require('diagnostics_channel');
+const http2 = require('http2');
+const { Duplex } = require('stream');
+
+let bodyChunkSent = false;
+
+dc.subscribe('http2.client.stream.bodyChunkSent', common.mustCall(({ stream, writev, data, encoding }) => {
+  // Since ClientHttp2Stream is not exported from any module, this just checks
+  // if the stream is an instance of Duplex.
+  assert.ok(stream instanceof Duplex);
+  assert.strictEqual(stream.constructor.name, 'ClientHttp2Stream');
+
+  assert.strictEqual(writev, true);
+
+  assert.ok(Array.isArray(data));
+  assert.strictEqual(data.length, 3);
+
+  assert.strictEqual(data[0].chunk, 'héllo');
+  assert.strictEqual(data[0].encoding, 'latin1');
+
+  assert.ok(Buffer.from('foo').equals(data[1].chunk));
+  assert.strictEqual(data[1].encoding, 'buffer');
+
+  assert.ok(Buffer.from('bar').equals(data[2].chunk));
+  assert.strictEqual(data[2].encoding, 'buffer');
+
+  assert.strictEqual(encoding, '');
+
+  bodyChunkSent = true;
+}));
+
+dc.subscribe('http2.client.stream.bodySent', common.mustCall(({ stream }) => {
+  // 'http2.client.stream.bodyChunkSent' must run first.
+  assert.ok(bodyChunkSent);
+
+  // Since ClientHttp2Stream is not exported from any module, this just checks
+  // if the stream is an instance of Duplex.
+  assert.ok(stream instanceof Duplex);
+  assert.strictEqual(stream.constructor.name, 'ClientHttp2Stream');
+}));
+
+const server = http2.createServer();
+server.on('stream', common.mustCall((stream) => {
+  stream.respond({}, { endStream: true });
+}));
+
+server.listen(0, common.mustCall(() => {
+  const port = server.address().port;
+  const client = http2.connect(`http://localhost:${port}`);
+
+  const stream = client.request({ [http2.constants.HTTP2_HEADER_METHOD]: 'POST' });
+  stream.write('héllo', 'latin1');
+  stream.write(Buffer.from('foo'));
+  stream.write(new TextEncoder().encode('bar'));
+  stream.end();
+
+  stream.on('response', common.mustCall(() => {
+    client.close();
+    server.close();
+  }));
+}, 1));

test/parallel/test-diagnostics-channel-http2-client-stream-body-multiple-buffers.js

@@ -0,0 +1,66 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+// This test ensures that the built-in HTTP/2 diagnostics channels are reporting
+// the diagnostics messages for the 'http2.client.stream.bodyChunkSent' and
+// 'http2.client.stream.bodySent' channels when ClientHttp2Streams bodies are
+// being sent with multiple Buffers.
+
+const assert = require('assert');
+const dc = require('diagnostics_channel');
+const http2 = require('http2');
+const { Duplex } = require('stream');
+
+let bodyChunkSent = false;
+
+dc.subscribe('http2.client.stream.bodyChunkSent', common.mustCall(({ stream, writev, data, encoding }) => {
+  // Since ClientHttp2Stream is not exported from any module, this just checks
+  // if the stream is an instance of Duplex.
+  assert.ok(stream instanceof Duplex);
+  assert.strictEqual(stream.constructor.name, 'ClientHttp2Stream');
+
+  assert.strictEqual(writev, true);
+
+  assert.ok(Array.isArray(data));
+  assert.strictEqual(data.length, 2);
+
+  assert.ok(Buffer.from('foo').equals(data[0]));
+  assert.ok(Buffer.from('bar').equals(data[1]));
+
+  assert.strictEqual(encoding, '');
+
+  bodyChunkSent = true;
+}));
+
+dc.subscribe('http2.client.stream.bodySent', common.mustCall(({ stream }) => {
+  // 'http2.client.stream.bodyChunkSent' must run first.
+  assert.ok(bodyChunkSent);
+
+  // Since ClientHttp2Stream is not exported from any module, this just checks
+  // if the stream is an instance of Duplex.
+  assert.ok(stream instanceof Duplex);
+  assert.strictEqual(stream.constructor.name, 'ClientHttp2Stream');
+}));
+
+const server = http2.createServer();
+server.on('stream', common.mustCall((stream) => {
+  stream.respond({}, { endStream: true });
+}));
+
+server.listen(0, common.mustCall(() => {
+  const port = server.address().port;
+  const client = http2.connect(`http://localhost:${port}`);
+
+  const stream = client.request({ [http2.constants.HTTP2_HEADER_METHOD]: 'POST' });
+  stream.write(Buffer.from('foo'));
+  stream.write(Buffer.from('bar'));
+  stream.end();
+
+  stream.on('response', common.mustCall(() => {
+    client.close();
+    server.close();
+  }));
+}, 1));

test/parallel/test-diagnostics-channel-http2-client-stream-body-no-chunks.js

@@ -0,0 +1,42 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+// This test ensures that the built-in HTTP/2 diagnostics channels are reporting
+// the diagnostics messages for the 'http2.client.stream.bodyChunkSent' and
+// 'http2.client.stream.bodySent' channels when ClientHttp2Streams bodies are
+// being sent with no chunks.
+
+const assert = require('assert');
+const dc = require('diagnostics_channel');
+const http2 = require('http2');
+const { Duplex } = require('stream');
+
+dc.subscribe('http2.client.stream.bodyChunkSent', common.mustNotCall());
+
+dc.subscribe('http2.client.stream.bodySent', common.mustCall(({ stream }) => {
+  // Since ClientHttp2Stream is not exported from any module, this just checks
+  // if the stream is an instance of Duplex.
+  assert.ok(stream instanceof Duplex);
+  assert.strictEqual(stream.constructor.name, 'ClientHttp2Stream');
+}));
+
+const server = http2.createServer();
+server.on('stream', common.mustCall((stream) => {
+  stream.respond({}, { endStream: true });
+}));
+
+server.listen(0, common.mustCall(() => {
+  const port = server.address().port;
+  const client = http2.connect(`http://localhost:${port}`);
+
+  const stream = client.request({ [http2.constants.HTTP2_HEADER_METHOD]: 'POST' });
+  stream.end();
+
+  stream.on('response', common.mustCall(() => {
+    client.close();
+    server.close();
+  }));
+}, 1));

test/parallel/test-diagnostics-channel-http2-client-stream-body-single-buffer.js

@@ -0,0 +1,59 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+// This test ensures that the built-in HTTP/2 diagnostics channels are reporting
+// the diagnostics messages for the 'http2.client.stream.bodyChunkSent' and
+// 'http2.client.stream.bodySent' channels when ClientHttp2Streams bodies are
+// being sent with a single Buffer.
+
+const assert = require('assert');
+const dc = require('diagnostics_channel');
+const http2 = require('http2');
+const { Duplex } = require('stream');
+
+let bodyChunkSent = false;
+
+dc.subscribe('http2.client.stream.bodyChunkSent', common.mustCall(({ stream, writev, data, encoding }) => {
+  // Since ClientHttp2Stream is not exported from any module, this just checks
+  // if the stream is an instance of Duplex.
+  assert.ok(stream instanceof Duplex);
+  assert.strictEqual(stream.constructor.name, 'ClientHttp2Stream');
+
+  assert.strictEqual(writev, false);
+  assert.ok(Buffer.from('foo').equals(data));
+  assert.strictEqual(encoding, 'buffer');
+
+  bodyChunkSent = true;
+}));
+
+dc.subscribe('http2.client.stream.bodySent', common.mustCall(({ stream }) => {
+  // 'http2.client.stream.bodyChunkSent' must run first.
+  assert.ok(bodyChunkSent);
+
+  // Since ClientHttp2Stream is not exported from any module, this just checks
+  // if the stream is an instance of Duplex.
+  assert.ok(stream instanceof Duplex);
+  assert.strictEqual(stream.constructor.name, 'ClientHttp2Stream');
+}));
+
+const server = http2.createServer();
+server.on('stream', common.mustCall((stream) => {
+  stream.respond({}, { endStream: true });
+}));
+
+server.listen(0, common.mustCall(() => {
+  const port = server.address().port;
+  const client = http2.connect(`http://localhost:${port}`);
+
+  const stream = client.request({ [http2.constants.HTTP2_HEADER_METHOD]: 'POST' });
+  stream.write(Buffer.from('foo'));
+  stream.end();
+
+  stream.on('response', common.mustCall(() => {
+    client.close();
+    server.close();
+  }));
+}, 1));

…tics-channel-http2-client-stream-body.js …ttp2-client-stream-body-single-string.jstest/parallel/test-diagnostics-channel-http2-client-stream-body.js renamed to test/parallel/test-diagnostics-channel-http2-client-stream-body-single-string.js test/parallel/test-diagnostics-channel-http2-client-stream-body.js renamed to test/parallel/test-diagnostics-channel-http2-client-stream-body-single-string.js

@@ -7,7 +7,7 @@ if (!common.hasCrypto)
 // This test ensures that the built-in HTTP/2 diagnostics channels are reporting
 // the diagnostics messages for the 'http2.client.stream.bodyChunkSent' and
 // 'http2.client.stream.bodySent' channels when ClientHttp2Streams bodies are
-// being sent.
+// being sent with a single string.
 
 const assert = require('assert');
 const dc = require('diagnostics_channel');
@@ -16,14 +16,15 @@ const { Duplex } = require('stream');
 
 let bodyChunkSent = false;
 
-dc.subscribe('http2.client.stream.bodyChunkSent', common.mustCall(({ stream, chunk }) => {
+dc.subscribe('http2.client.stream.bodyChunkSent', common.mustCall(({ stream, writev, data, encoding }) => {
   // Since ClientHttp2Stream is not exported from any module, this just checks
   // if the stream is an instance of Duplex.
   assert.ok(stream instanceof Duplex);
   assert.strictEqual(stream.constructor.name, 'ClientHttp2Stream');
 
-  assert.strictEqual(Buffer.isBuffer(chunk), true);
-  assert.strictEqual(chunk.toString(), 'foobarbaz');
+  assert.strictEqual(writev, false);
+  assert.strictEqual(data, 'foo');
+  assert.strictEqual(encoding, 'utf8');
 
   bodyChunkSent = true;
 }));
@@ -49,8 +50,6 @@ server.listen(0, common.mustCall(() => {
 
   const stream = client.request({ [http2.constants.HTTP2_HEADER_METHOD]: 'POST' });
   stream.write('foo');
-  stream.write(Buffer.from('bar'));
-  stream.write(new TextEncoder().encode('baz'));
   stream.end();
 
   stream.on('response', common.mustCall(() => {