From a5b6677aee1da29f403bfa03eb42c0a0faf10630 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Thu, 7 Sep 2017 13:20:37 -0400 Subject: [PATCH] http2: set decodeStrings to false, test Set writableStream decodeStrings to false to let the native layer handle converting strings to buffer. PR-URL: https://github.com/nodejs/node/pull/15140 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Claudio Rodriguez Reviewed-By: Matteo Collina Reviewed-By: James M Snell --- benchmark/http2/write.js | 28 +++++++++ lib/internal/http2/core.js | 11 ++-- test/parallel/test-http2-createwritereq.js | 68 ++++++++++++++++++++++ 3 files changed, 102 insertions(+), 5 deletions(-) create mode 100644 benchmark/http2/write.js create mode 100644 test/parallel/test-http2-createwritereq.js diff --git a/benchmark/http2/write.js b/benchmark/http2/write.js new file mode 100644 index 00000000000000..df76794468b4de --- /dev/null +++ b/benchmark/http2/write.js @@ -0,0 +1,28 @@ +'use strict'; + +const common = require('../common.js'); +const PORT = common.PORT; + +var bench = common.createBenchmark(main, { + streams: [100, 200, 1000], + length: [64 * 1024, 128 * 1024, 256 * 1024, 1024 * 1024], +}, { flags: ['--expose-http2', '--no-warnings'] }); + +function main(conf) { + const m = +conf.streams; + const l = +conf.length; + const http2 = require('http2'); + const server = http2.createServer(); + server.on('stream', (stream) => { + stream.respond(); + stream.write('ΓΌ'.repeat(l)); + stream.end(); + }); + server.listen(PORT, () => { + bench.http({ + path: '/', + requests: 10000, + maxConcurrentStreams: m, + }, () => { server.close(); }); + }); +} diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 9e698d8874f8ae..8646f843a5bc88 100755 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -1161,11 +1161,6 @@ class ClientHttp2Session extends Http2Session { function createWriteReq(req, handle, data, encoding) { switch (encoding) { - case 'latin1': - case 'binary': - return handle.writeLatin1String(req, data); - case 'buffer': - return handle.writeBuffer(req, data); case 'utf8': case 'utf-8': return handle.writeUtf8String(req, data); @@ -1176,6 +1171,11 @@ function createWriteReq(req, handle, data, encoding) { case 'utf16le': case 'utf-16le': return handle.writeUcs2String(req, data); + case 'latin1': + case 'binary': + return handle.writeLatin1String(req, data); + case 'buffer': + return handle.writeBuffer(req, data); default: return handle.writeBuffer(req, Buffer.from(data, encoding)); } @@ -1287,6 +1287,7 @@ function abort(stream) { class Http2Stream extends Duplex { constructor(session, options) { options.allowHalfOpen = true; + options.decodeStrings = false; super(options); this.cork(); this[kSession] = session; diff --git a/test/parallel/test-http2-createwritereq.js b/test/parallel/test-http2-createwritereq.js new file mode 100644 index 00000000000000..7e4bd5cf112ee9 --- /dev/null +++ b/test/parallel/test-http2-createwritereq.js @@ -0,0 +1,68 @@ +// Flags: --expose-http2 +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const assert = require('assert'); +const http2 = require('http2'); + +// Tests that write uses the correct encoding when writing +// using the helper function createWriteReq + +const testString = 'a\u00A1\u0100\uD83D\uDE00'; + +const encodings = { + 'buffer': 'utf8', + 'ascii': 'ascii', + 'latin1': 'latin1', + 'binary': 'latin1', + 'utf8': 'utf8', + 'utf-8': 'utf8', + 'ucs2': 'ucs2', + 'ucs-2': 'ucs2', + 'utf16le': 'ucs2', + 'utf-16le': 'ucs2', + 'UTF8': 'utf8' // should fall through to Buffer.from +}; + +const testsToRun = Object.keys(encodings).length; +let testsFinished = 0; + +const server = http2.createServer(common.mustCall((req, res) => { + const testEncoding = encodings[req.path.slice(1)]; + + req.on('data', common.mustCall((chunk) => assert.ok( + Buffer.from(testString, testEncoding).equals(chunk) + ))); + + req.on('end', () => res.end()); +}, Object.keys(encodings).length)); + +server.listen(0, common.mustCall(function() { + Object.keys(encodings).forEach((writeEncoding) => { + const client = http2.connect(`http://localhost:${this.address().port}`); + const req = client.request({ + ':path': `/${writeEncoding}`, + ':method': 'POST' + }); + + assert.strictEqual(req._writableState.decodeStrings, false); + req.write( + writeEncoding !== 'buffer' ? testString : Buffer.from(testString), + writeEncoding !== 'buffer' ? writeEncoding : undefined + ); + req.resume(); + + req.on('end', common.mustCall(function() { + client.destroy(); + testsFinished++; + + if (testsFinished === testsToRun) { + server.close(); + } + })); + + req.end(); + }); +}));