Skip to content

Commit 2f5f6ed

Browse files
apapirovskiaddaleax
authored andcommitted
http2: set decodeStrings to false, test
Set writableStream decodeStrings to false to let the native layer handle converting strings to buffer. PR-URL: nodejs#15140 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Claudio Rodriguez <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 993faa7 commit 2f5f6ed

File tree

3 files changed

+102
-5
lines changed

3 files changed

+102
-5
lines changed

benchmark/http2/write.js

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
'use strict';
2+
3+
const common = require('../common.js');
4+
const PORT = common.PORT;
5+
6+
var bench = common.createBenchmark(main, {
7+
streams: [100, 200, 1000],
8+
length: [64 * 1024, 128 * 1024, 256 * 1024, 1024 * 1024],
9+
}, { flags: ['--expose-http2', '--no-warnings'] });
10+
11+
function main(conf) {
12+
const m = +conf.streams;
13+
const l = +conf.length;
14+
const http2 = require('http2');
15+
const server = http2.createServer();
16+
server.on('stream', (stream) => {
17+
stream.respond();
18+
stream.write('ü'.repeat(l));
19+
stream.end();
20+
});
21+
server.listen(PORT, () => {
22+
bench.http({
23+
path: '/',
24+
requests: 10000,
25+
maxConcurrentStreams: m,
26+
}, () => { server.close(); });
27+
});
28+
}

lib/internal/http2/core.js

+6-5
Original file line numberDiff line numberDiff line change
@@ -1161,11 +1161,6 @@ class ClientHttp2Session extends Http2Session {
11611161

11621162
function createWriteReq(req, handle, data, encoding) {
11631163
switch (encoding) {
1164-
case 'latin1':
1165-
case 'binary':
1166-
return handle.writeLatin1String(req, data);
1167-
case 'buffer':
1168-
return handle.writeBuffer(req, data);
11691164
case 'utf8':
11701165
case 'utf-8':
11711166
return handle.writeUtf8String(req, data);
@@ -1176,6 +1171,11 @@ function createWriteReq(req, handle, data, encoding) {
11761171
case 'utf16le':
11771172
case 'utf-16le':
11781173
return handle.writeUcs2String(req, data);
1174+
case 'latin1':
1175+
case 'binary':
1176+
return handle.writeLatin1String(req, data);
1177+
case 'buffer':
1178+
return handle.writeBuffer(req, data);
11791179
default:
11801180
return handle.writeBuffer(req, Buffer.from(data, encoding));
11811181
}
@@ -1287,6 +1287,7 @@ function abort(stream) {
12871287
class Http2Stream extends Duplex {
12881288
constructor(session, options) {
12891289
options.allowHalfOpen = true;
1290+
options.decodeStrings = false;
12901291
super(options);
12911292
this.cork();
12921293
this[kSession] = session;
+68
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
// Flags: --expose-http2
2+
'use strict';
3+
4+
const common = require('../common');
5+
if (!common.hasCrypto)
6+
common.skip('missing crypto');
7+
const assert = require('assert');
8+
const http2 = require('http2');
9+
10+
// Tests that write uses the correct encoding when writing
11+
// using the helper function createWriteReq
12+
13+
const testString = 'a\u00A1\u0100\uD83D\uDE00';
14+
15+
const encodings = {
16+
'buffer': 'utf8',
17+
'ascii': 'ascii',
18+
'latin1': 'latin1',
19+
'binary': 'latin1',
20+
'utf8': 'utf8',
21+
'utf-8': 'utf8',
22+
'ucs2': 'ucs2',
23+
'ucs-2': 'ucs2',
24+
'utf16le': 'ucs2',
25+
'utf-16le': 'ucs2',
26+
'UTF8': 'utf8' // should fall through to Buffer.from
27+
};
28+
29+
const testsToRun = Object.keys(encodings).length;
30+
let testsFinished = 0;
31+
32+
const server = http2.createServer(common.mustCall((req, res) => {
33+
const testEncoding = encodings[req.path.slice(1)];
34+
35+
req.on('data', common.mustCall((chunk) => assert.ok(
36+
Buffer.from(testString, testEncoding).equals(chunk)
37+
)));
38+
39+
req.on('end', () => res.end());
40+
}, Object.keys(encodings).length));
41+
42+
server.listen(0, common.mustCall(function() {
43+
Object.keys(encodings).forEach((writeEncoding) => {
44+
const client = http2.connect(`http://localhost:${this.address().port}`);
45+
const req = client.request({
46+
':path': `/${writeEncoding}`,
47+
':method': 'POST'
48+
});
49+
50+
assert.strictEqual(req._writableState.decodeStrings, false);
51+
req.write(
52+
writeEncoding !== 'buffer' ? testString : Buffer.from(testString),
53+
writeEncoding !== 'buffer' ? writeEncoding : undefined
54+
);
55+
req.resume();
56+
57+
req.on('end', common.mustCall(function() {
58+
client.destroy();
59+
testsFinished++;
60+
61+
if (testsFinished === testsToRun) {
62+
server.close();
63+
}
64+
}));
65+
66+
req.end();
67+
});
68+
}));

0 commit comments

Comments
 (0)