Skip to content

Commit

Permalink
http2: set decodeStrings to false, test
Browse files Browse the repository at this point in the history
Set writableStream decodeStrings to false to let the
native layer handle converting strings to buffer.

PR-URL: #15140
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
apapirovski authored and MylesBorins committed Sep 12, 2017
1 parent ac71d99 commit 4882f07
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 5 deletions.
28 changes: 28 additions & 0 deletions benchmark/http2/write.js
Original file line number Diff line number Diff line change
@@ -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(); });
});
}
11 changes: 6 additions & 5 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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));
}
Expand Down Expand Up @@ -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;
Expand Down
68 changes: 68 additions & 0 deletions test/parallel/test-http2-createwritereq.js
Original file line number Diff line number Diff line change
@@ -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();
});
}));

0 comments on commit 4882f07

Please sign in to comment.