Skip to content
This repository was archived by the owner on Apr 22, 2023. It is now read-only.

Commit 6d3d60a

Browse files
committed
http: Write hex/base64 chunks properly
This removes a dubious performance "optimization" where strings body chunks were concatenated to one another (and to the headers) without any regard for their encoding.
1 parent dce02a1 commit 6d3d60a

File tree

2 files changed

+63
-24
lines changed

2 files changed

+63
-24
lines changed

lib/_http_outgoing.js

+10-24
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,9 @@ OutgoingMessage.prototype._send = function(data, encoding) {
115115
// the same packet. Future versions of Node are going to take care of
116116
// this at a lower level and in a more general way.
117117
if (!this._headerSent) {
118-
if (util.isString(data)) {
118+
if (util.isString(data) &&
119+
encoding !== 'hex' &&
120+
encoding !== 'base64') {
119121
data = this._header + data;
120122
} else {
121123
this.output.unshift(this._header);
@@ -162,25 +164,6 @@ OutgoingMessage.prototype._writeRaw = function(data, encoding) {
162164

163165

164166
OutgoingMessage.prototype._buffer = function(data, encoding) {
165-
if (data.length === 0) return;
166-
167-
var length = this.output.length;
168-
169-
if (length === 0 || !util.isString(data)) {
170-
this.output.push(data);
171-
this.outputEncodings.push(encoding);
172-
return false;
173-
}
174-
175-
var lastEncoding = this.outputEncodings[length - 1];
176-
var lastData = this.output[length - 1];
177-
178-
if ((encoding && lastEncoding === encoding) ||
179-
(!encoding && data.constructor === lastData.constructor)) {
180-
this.output[length - 1] = lastData + data;
181-
return false;
182-
}
183-
184167
this.output.push(data);
185168
this.outputEncodings.push(encoding);
186169

@@ -421,13 +404,16 @@ OutgoingMessage.prototype.write = function(chunk, encoding) {
421404
ret = this._send(chunk, encoding);
422405
} else {
423406
// buffer, or a non-toString-friendly encoding
424-
len = chunk.length;
407+
if (util.isString(chunk))
408+
len = Buffer.byteLength(chunk, encoding);
409+
else
410+
len = chunk.length;
425411

426412
if (this.connection)
427413
this.connection.cork();
428-
this._send(len.toString(16));
414+
this._send(len.toString(16), 'ascii');
429415
this._send(crlf_buf);
430-
this._send(chunk);
416+
this._send(chunk, encoding);
431417
ret = this._send(crlf_buf);
432418
if (this.connection)
433419
this.connection.uncork();
@@ -493,7 +479,7 @@ OutgoingMessage.prototype.end = function(data, encoding) {
493479
}
494480

495481
if (this.chunkedEncoding) {
496-
ret = this._send('0\r\n' + this._trailer + '\r\n'); // Last chunk.
482+
ret = this._send('0\r\n' + this._trailer + '\r\n', 'ascii');
497483
} else {
498484
// Force a flush, HACK.
499485
ret = this._send('');

test/simple/test-http-hex-write.js

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Copyright Joyent, Inc. and other Node contributors.
2+
//
3+
// Permission is hereby granted, free of charge, to any person obtaining a
4+
// copy of this software and associated documentation files (the
5+
// "Software"), to deal in the Software without restriction, including
6+
// without limitation the rights to use, copy, modify, merge, publish,
7+
// distribute, sublicense, and/or sell copies of the Software, and to permit
8+
// persons to whom the Software is furnished to do so, subject to the
9+
// following conditions:
10+
//
11+
// The above copyright notice and this permission notice shall be included
12+
// in all copies or substantial portions of the Software.
13+
//
14+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
15+
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
16+
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
17+
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
18+
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
19+
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
20+
// USE OR OTHER DEALINGS IN THE SOFTWARE.
21+
22+
var common = require('../common');
23+
var assert = require('assert');
24+
25+
var http = require('http');
26+
27+
var expect = 'hex\nutf8\n';
28+
var data = '';
29+
var ended = false;
30+
31+
process.on('exit', function() {
32+
assert(ended);
33+
assert.equal(data, expect);
34+
console.log('ok');
35+
});
36+
37+
http.createServer(function(q, s) {
38+
s.setHeader('content-length', expect.length);
39+
s.write('6865780a', 'hex');
40+
s.write('utf8\n');
41+
s.end();
42+
this.close();
43+
}).listen(common.PORT, function() {
44+
http.request({ port: common.PORT }).on('response', function(res) {
45+
res.setEncoding('ascii');
46+
res.on('data', function(c) {
47+
data += c;
48+
});
49+
res.on('end', function() {
50+
ended = true;
51+
});
52+
}).end();
53+
});

0 commit comments

Comments
 (0)