From a2a954b34a98fdd42cb5599d4324b0687925f91d Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Mon, 6 Feb 2023 17:57:57 +0100 Subject: [PATCH 1/2] http: unify header treatment --- lib/_http_outgoing.js | 2 +- .../test-http-server-non-utf8-header.js | 48 +++++++++++++++++++ .../test-http-server-response-standalone.js | 6 +-- 3 files changed, 51 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-http-server-non-utf8-header.js diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index ea3d7e62556212..169b0c8112e498 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -359,7 +359,7 @@ OutgoingMessage.prototype._send = function _send(data, encoding, callback) { // `this._header` can be null if OutgoingMessage is used without a proper Socket // See: /test/parallel/test-http-outgoing-message-inheritance.js if (typeof data === 'string' && - (encoding === 'utf8' || encoding === 'latin1' || !encoding)) { + (encoding === 'utf8' || encoding === 'latin1')) { data = this._header + data; } else { const header = this._header; diff --git a/test/parallel/test-http-server-non-utf8-header.js b/test/parallel/test-http-server-non-utf8-header.js new file mode 100644 index 00000000000000..f6c0a7e7506946 --- /dev/null +++ b/test/parallel/test-http-server-non-utf8-header.js @@ -0,0 +1,48 @@ +'use strict'; +const common = require('../common'); +const http = require('http'); +const assert = require('assert'); + +const nonUtf8Header = 'bår'; +const nonUtf8ToLatin1 = Buffer.from(nonUtf8Header).toString('latin1'); + +{ + const server = http.createServer(common.mustCall((req, res) => { + res.writeHead(200, [ + 'foo', + Buffer.from(nonUtf8Header).toString('binary'), + ]); + res.end('hello'); + })); + + server.listen(0, common.mustCall(() => { + http.get({ port: server.address().port }, (res) => { + assert.strictEqual(res.statusCode, 200); + assert.strictEqual(res.headers.foo, nonUtf8ToLatin1); + res.resume().on('end', common.mustCall(() => { + server.close(); + })); + }); + })); +} + +{ + const server = http.createServer(common.mustCall((req, res) => { + res.writeHead(200, [ + 'Content-Length', '5', + 'foo', + Buffer.from(nonUtf8Header).toString('binary'), + ]); + res.end('hello'); + })); + + server.listen(0, common.mustCall(() => { + http.get({ port: server.address().port }, (res) => { + assert.strictEqual(res.statusCode, 200); + assert.strictEqual(res.headers.foo, nonUtf8ToLatin1); + res.resume().on('end', common.mustCall(() => { + server.close(); + })); + }); + })); +} diff --git a/test/parallel/test-http-server-response-standalone.js b/test/parallel/test-http-server-response-standalone.js index ec6d1e89e38525..68f037b3e3a9a6 100644 --- a/test/parallel/test-http-server-response-standalone.js +++ b/test/parallel/test-http-server-response-standalone.js @@ -20,13 +20,11 @@ let firstChunk = true; const ws = new Writable({ write: common.mustCall((chunk, encoding, callback) => { if (firstChunk) { - assert(chunk.toString().endsWith('hello world')); + assert(chunk.toString().startsWith('HTTP/1.1 200 OK')); firstChunk = false; - } else { - assert.strictEqual(chunk.length, 0); } setImmediate(callback); - }, 2) + }, 3) }); res.assignSocket(ws); From 89d6e2593cc888c131b390926de797a38afe8f27 Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Mon, 6 Feb 2023 23:13:46 +0100 Subject: [PATCH 2/2] http: processHeader check isContentDispositionField --- lib/_http_outgoing.js | 15 ++++++++++++++- test/parallel/test-http-server-non-utf8-header.js | 8 ++++---- .../test-http-server-response-standalone.js | 6 ++++-- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 169b0c8112e498..fd853f8d51213b 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -100,6 +100,10 @@ function isCookieField(s) { return s.length === 6 && StringPrototypeToLowerCase(s) === 'cookie'; } +function isContentDispositionField(s) { + return s.length === 19 && StringPrototypeToLowerCase(s) === 'content-disposition'; +} + function OutgoingMessage() { Stream.call(this); @@ -359,7 +363,7 @@ OutgoingMessage.prototype._send = function _send(data, encoding, callback) { // `this._header` can be null if OutgoingMessage is used without a proper Socket // See: /test/parallel/test-http-outgoing-message-inheritance.js if (typeof data === 'string' && - (encoding === 'utf8' || encoding === 'latin1')) { + (encoding === 'utf8' || encoding === 'latin1' || !encoding)) { data = this._header + data; } else { const header = this._header; @@ -570,6 +574,15 @@ function _storeHeader(firstLine, headers) { function processHeader(self, state, key, value, validate) { if (validate) validateHeaderName(key); + + // If key is content-disposition and there is content-length + // encode the value in latin1 + // https://www.rfc-editor.org/rfc/rfc6266#section-4.3 + // Refs: https://github.com/nodejs/node/pull/46528 + if (isContentDispositionField(key) && self._contentLength) { + value = Buffer.from(value, 'latin1'); + } + if (ArrayIsArray(value)) { if ( (value.length < 2 || !isCookieField(key)) && diff --git a/test/parallel/test-http-server-non-utf8-header.js b/test/parallel/test-http-server-non-utf8-header.js index f6c0a7e7506946..331965ae38d0f8 100644 --- a/test/parallel/test-http-server-non-utf8-header.js +++ b/test/parallel/test-http-server-non-utf8-header.js @@ -9,7 +9,7 @@ const nonUtf8ToLatin1 = Buffer.from(nonUtf8Header).toString('latin1'); { const server = http.createServer(common.mustCall((req, res) => { res.writeHead(200, [ - 'foo', + 'content-disposition', Buffer.from(nonUtf8Header).toString('binary'), ]); res.end('hello'); @@ -18,7 +18,7 @@ const nonUtf8ToLatin1 = Buffer.from(nonUtf8Header).toString('latin1'); server.listen(0, common.mustCall(() => { http.get({ port: server.address().port }, (res) => { assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers.foo, nonUtf8ToLatin1); + assert.strictEqual(res.headers['content-disposition'], nonUtf8ToLatin1); res.resume().on('end', common.mustCall(() => { server.close(); })); @@ -30,7 +30,7 @@ const nonUtf8ToLatin1 = Buffer.from(nonUtf8Header).toString('latin1'); const server = http.createServer(common.mustCall((req, res) => { res.writeHead(200, [ 'Content-Length', '5', - 'foo', + 'content-disposition', Buffer.from(nonUtf8Header).toString('binary'), ]); res.end('hello'); @@ -39,7 +39,7 @@ const nonUtf8ToLatin1 = Buffer.from(nonUtf8Header).toString('latin1'); server.listen(0, common.mustCall(() => { http.get({ port: server.address().port }, (res) => { assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers.foo, nonUtf8ToLatin1); + assert.strictEqual(res.headers['content-disposition'], nonUtf8ToLatin1); res.resume().on('end', common.mustCall(() => { server.close(); })); diff --git a/test/parallel/test-http-server-response-standalone.js b/test/parallel/test-http-server-response-standalone.js index 68f037b3e3a9a6..ec6d1e89e38525 100644 --- a/test/parallel/test-http-server-response-standalone.js +++ b/test/parallel/test-http-server-response-standalone.js @@ -20,11 +20,13 @@ let firstChunk = true; const ws = new Writable({ write: common.mustCall((chunk, encoding, callback) => { if (firstChunk) { - assert(chunk.toString().startsWith('HTTP/1.1 200 OK')); + assert(chunk.toString().endsWith('hello world')); firstChunk = false; + } else { + assert.strictEqual(chunk.length, 0); } setImmediate(callback); - }, 3) + }, 2) }); res.assignSocket(ws);