From 0c74f9c9b36a0939e82dd70d0665f7bba84d377b Mon Sep 17 00:00:00 2001 From: KaKa Date: Tue, 7 Feb 2023 19:10:40 +0800 Subject: [PATCH 1/9] fix: content-disposition header parsing --- lib/client.js | 6 ++++ test/issue-1903.js | 78 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 test/issue-1903.js diff --git a/lib/client.js b/lib/client.js index 983e7034795..8efb28c3c24 100644 --- a/lib/client.js +++ b/lib/client.js @@ -4,6 +4,7 @@ const assert = require('assert') const net = require('net') +const transcode = require('buffer').transcode const util = require('./core/util') const timers = require('./timers') const Request = require('./core/request') @@ -618,6 +619,11 @@ class Parser { this.connection += buf.toString() } else if (key.length === 14 && key.toString().toLowerCase() === 'content-length') { this.contentLength += buf.toString() + } else if (key.length === 19 && key.toString().toLowerCase() === 'content-disposition') { + // https://www.rfc-editor.org/rfc/rfc6266#section-4.3 + // we need to recieve the buffer as latin1 + // so it would not transform to utf-8 wrongly + this.headers[len - 1] = transcode(buf, 'latin1', 'utf8') } this.trackHeader(buf.length) diff --git a/test/issue-1903.js b/test/issue-1903.js new file mode 100644 index 00000000000..c8fa644dbac --- /dev/null +++ b/test/issue-1903.js @@ -0,0 +1,78 @@ +'use strict' + +const { createServer } = require('node:http') +const { test } = require('tap') +const { request } = require('..') + +function createPromise() { + const result = {} + result.promise = new Promise((resolve) => { + result.resolve = resolve + }) + return result +} + +test('should parse content-disposition consistencely', async (t) => { + t.plan(5) + + // create promise to allow server spinup in parallel + const spinup1 = createPromise() + const spinup2 = createPromise() + const spinup3 = createPromise() + + // variables to store content-disposition header + const header = [] + + const server = createServer((req, res) => { + res.writeHead(200, { + 'content-length': 2, + 'content-disposition': 'attachment; filename="år.pdf"' + }) + header.push(header) + res.end('OK', spinup1.resolve) + }) + t.teardown(server.close.bind(server)) + server.listen(0, spinup1.resolve) + + const proxy1 = createServer(async (req, res) => { + const { statusCode, headers, body } = await request(`http://localhost:${server.address().port}`, { + method: "GET" + }) + header.push(headers['content-disposition']) + delete headers['transfer-encoding'] + res.writeHead(statusCode, headers) + body.pipe(res) + }) + t.teardown(proxy1.close.bind(proxy1)) + proxy1.listen(0, spinup2.resolve) + + const proxy2 = createServer(async (req, res) => { + const { statusCode, headers, body } = await request(`http://localhost:${proxy1.address().port}`, { + method: "GET" + }) + header.push(headers['content-disposition']) + delete headers['transfer-encoding'] + res.writeHead(statusCode, headers) + body.pipe(res) + }) + t.teardown(proxy2.close.bind(proxy2)) + proxy2.listen(0, spinup3.resolve) + + // wait until all server spinup + await Promise.all([spinup1.promise, spinup2.promise, spinup3.promise]) + + const { statusCode, headers, body } = await request(`http://localhost:${proxy2.address().port}`, { + method: "GET" + }) + header.push(headers['content-disposition']) + t.equal(statusCode, 200) + t.equal(await body.text(), 'OK') + + // we check header + // must not be the same in first proxy + t.notSame(header[0], header[1]) + // chaining always the same value + t.equal(header[1], header[2]) + t.equal(header[2], header[3]) +}) + From 90de7f3cdc46c788d4e260a4e776dac84d354dbf Mon Sep 17 00:00:00 2001 From: KaKa Date: Tue, 7 Feb 2023 20:11:41 +0800 Subject: [PATCH 2/9] test: fix the first header --- test/issue-1903.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/issue-1903.js b/test/issue-1903.js index c8fa644dbac..a5634e1d317 100644 --- a/test/issue-1903.js +++ b/test/issue-1903.js @@ -28,7 +28,7 @@ test('should parse content-disposition consistencely', async (t) => { 'content-length': 2, 'content-disposition': 'attachment; filename="år.pdf"' }) - header.push(header) + header.push('attachment; filename="år.pdf"') res.end('OK', spinup1.resolve) }) t.teardown(server.close.bind(server)) From 73bad73481205a4edd866d9cbaa96d9907d1bc34 Mon Sep 17 00:00:00 2001 From: KaKa Date: Tue, 7 Feb 2023 20:12:19 +0800 Subject: [PATCH 3/9] refactor: update import Co-authored-by: Robert Nagy --- lib/client.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/client.js b/lib/client.js index 8efb28c3c24..f6ecf20d568 100644 --- a/lib/client.js +++ b/lib/client.js @@ -4,7 +4,7 @@ const assert = require('assert') const net = require('net') -const transcode = require('buffer').transcode +const Buffer = require('buffer') const util = require('./core/util') const timers = require('./timers') const Request = require('./core/request') @@ -623,7 +623,7 @@ class Parser { // https://www.rfc-editor.org/rfc/rfc6266#section-4.3 // we need to recieve the buffer as latin1 // so it would not transform to utf-8 wrongly - this.headers[len - 1] = transcode(buf, 'latin1', 'utf8') + this.headers[len - 1] = Buffer.transcode(buf, 'latin1', 'utf8') } this.trackHeader(buf.length) From 5ff9962fb1d334ba090739b6072af95509fb525a Mon Sep 17 00:00:00 2001 From: KaKa Date: Tue, 7 Feb 2023 20:38:38 +0800 Subject: [PATCH 4/9] refactor: transcode inside onHeadersComplete --- lib/client.js | 14 ++++++++------ test/issue-1903.js | 11 +++++------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/client.js b/lib/client.js index f6ecf20d568..fe8d7447bd2 100644 --- a/lib/client.js +++ b/lib/client.js @@ -4,7 +4,7 @@ const assert = require('assert') const net = require('net') -const Buffer = require('buffer') +const { transcode } = require('buffer') const util = require('./core/util') const timers = require('./timers') const Request = require('./core/request') @@ -619,11 +619,6 @@ class Parser { this.connection += buf.toString() } else if (key.length === 14 && key.toString().toLowerCase() === 'content-length') { this.contentLength += buf.toString() - } else if (key.length === 19 && key.toString().toLowerCase() === 'content-disposition') { - // https://www.rfc-editor.org/rfc/rfc6266#section-4.3 - // we need to recieve the buffer as latin1 - // so it would not transform to utf-8 wrongly - this.headers[len - 1] = Buffer.transcode(buf, 'latin1', 'utf8') } this.trackHeader(buf.length) @@ -772,6 +767,13 @@ class Parser { let pause try { + const idx = headers.findIndex((v) => v.length === 19 && v.toString().toLowerCase() === 'content-disposition') + if (idx > -1) { + // https://www.rfc-editor.org/rfc/rfc6266#section-4.3 + // we need to recieve the buffer as latin1 + // so it would not transform to utf-8 wrongly + headers[idx + 1] = transcode(headers[idx + 1], 'latin1', 'utf8') + } pause = request.onHeaders(statusCode, headers, this.resume, statusText) === false } catch (err) { util.destroy(socket, err) diff --git a/test/issue-1903.js b/test/issue-1903.js index a5634e1d317..9902cb43970 100644 --- a/test/issue-1903.js +++ b/test/issue-1903.js @@ -26,9 +26,9 @@ test('should parse content-disposition consistencely', async (t) => { const server = createServer((req, res) => { res.writeHead(200, { 'content-length': 2, - 'content-disposition': 'attachment; filename="år.pdf"' + 'content-disposition': "attachment; filename='år.pdf'" }) - header.push('attachment; filename="år.pdf"') + header.push("attachment; filename='år.pdf'") res.end('OK', spinup1.resolve) }) t.teardown(server.close.bind(server)) @@ -36,7 +36,7 @@ test('should parse content-disposition consistencely', async (t) => { const proxy1 = createServer(async (req, res) => { const { statusCode, headers, body } = await request(`http://localhost:${server.address().port}`, { - method: "GET" + method: 'GET' }) header.push(headers['content-disposition']) delete headers['transfer-encoding'] @@ -48,7 +48,7 @@ test('should parse content-disposition consistencely', async (t) => { const proxy2 = createServer(async (req, res) => { const { statusCode, headers, body } = await request(`http://localhost:${proxy1.address().port}`, { - method: "GET" + method: 'GET' }) header.push(headers['content-disposition']) delete headers['transfer-encoding'] @@ -62,7 +62,7 @@ test('should parse content-disposition consistencely', async (t) => { await Promise.all([spinup1.promise, spinup2.promise, spinup3.promise]) const { statusCode, headers, body } = await request(`http://localhost:${proxy2.address().port}`, { - method: "GET" + method: 'GET' }) header.push(headers['content-disposition']) t.equal(statusCode, 200) @@ -75,4 +75,3 @@ test('should parse content-disposition consistencely', async (t) => { t.equal(header[1], header[2]) t.equal(header[2], header[3]) }) - From 6a9b0aca635d68327899ff01fb129d981de88df4 Mon Sep 17 00:00:00 2001 From: KaKa Date: Tue, 7 Feb 2023 20:39:13 +0800 Subject: [PATCH 5/9] refactor: linting --- test/issue-1903.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/issue-1903.js b/test/issue-1903.js index 9902cb43970..42d8f2a3990 100644 --- a/test/issue-1903.js +++ b/test/issue-1903.js @@ -4,7 +4,7 @@ const { createServer } = require('node:http') const { test } = require('tap') const { request } = require('..') -function createPromise() { +function createPromise () { const result = {} result.promise = new Promise((resolve) => { result.resolve = resolve From 17b8995856823e3d6da228410984f555acc84330 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 7 Feb 2023 13:47:09 +0100 Subject: [PATCH 6/9] fixup --- lib/client.js | 8 -------- lib/core/util.js | 23 ++++++++++++++++++++--- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/lib/client.js b/lib/client.js index fe8d7447bd2..983e7034795 100644 --- a/lib/client.js +++ b/lib/client.js @@ -4,7 +4,6 @@ const assert = require('assert') const net = require('net') -const { transcode } = require('buffer') const util = require('./core/util') const timers = require('./timers') const Request = require('./core/request') @@ -767,13 +766,6 @@ class Parser { let pause try { - const idx = headers.findIndex((v) => v.length === 19 && v.toString().toLowerCase() === 'content-disposition') - if (idx > -1) { - // https://www.rfc-editor.org/rfc/rfc6266#section-4.3 - // we need to recieve the buffer as latin1 - // so it would not transform to utf-8 wrongly - headers[idx + 1] = transcode(headers[idx + 1], 'latin1', 'utf8') - } pause = request.onHeaders(statusCode, headers, this.resume, statusText) === false } catch (err) { util.destroy(socket, err) diff --git a/lib/core/util.js b/lib/core/util.js index 6f38ffc8a4d..7da78a04840 100644 --- a/lib/core/util.js +++ b/lib/core/util.js @@ -213,25 +213,42 @@ function parseHeaders (headers, obj = {}) { for (let i = 0; i < headers.length; i += 2) { const key = headers[i].toString().toLowerCase() let val = obj[key] + + const encoding = key.length === 19 && key === 'content-disposition' + ? 'latin1' + : 'utf8' + if (!val) { if (Array.isArray(headers[i + 1])) { obj[key] = headers[i + 1] } else { - obj[key] = headers[i + 1].toString() + obj[key] = headers[i + 1].toString(encoding) } } else { if (!Array.isArray(val)) { val = [val] obj[key] = val } - val.push(headers[i + 1].toString()) + val.push(headers[i + 1].toString(encoding)) } } return obj } function parseRawHeaders (headers) { - return headers.map(header => header.toString()) + const ret = [] + for (let n = 0; n < headers.length; n += 2) { + const key = headers[n + 0].toString() + + const encoding = key.length === 19 && key.toLowerCase() === 'content-disposition' + ? 'latin1' + : 'utf8' + + const val = headers[n + 1].toString(encoding) + + headers.push(key, val) + } + return ret } function isBuffer (buffer) { From 8c6ec839c73def0c2e9f3296f43a853a0970dea5 Mon Sep 17 00:00:00 2001 From: KaKa Date: Tue, 7 Feb 2023 20:53:47 +0800 Subject: [PATCH 7/9] fixup: infinite loop --- lib/core/util.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/core/util.js b/lib/core/util.js index 7da78a04840..3b9b56cb64d 100644 --- a/lib/core/util.js +++ b/lib/core/util.js @@ -246,7 +246,7 @@ function parseRawHeaders (headers) { const val = headers[n + 1].toString(encoding) - headers.push(key, val) + ret.push(key, val) } return ret } From 34e36332d7f8564fcd94c8838ba0fbe3e0e032c3 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 7 Feb 2023 16:04:22 +0100 Subject: [PATCH 8/9] fixup --- lib/mock/mock-utils.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/mock/mock-utils.js b/lib/mock/mock-utils.js index 9dc414d0a35..42ea185cc0e 100644 --- a/lib/mock/mock-utils.js +++ b/lib/mock/mock-utils.js @@ -188,7 +188,11 @@ function buildKey (opts) { } function generateKeyValues (data) { - return Object.entries(data).reduce((keyValuePairs, [key, value]) => [...keyValuePairs, key, value], []) + return Object.entries(data).reduce((keyValuePairs, [key, value]) => [ + ...keyValuePairs, + Buffer.from(`${key}`), + Array.isArray(value) ? value.map(x => Buffer.from(`${x}`)) : Buffer.from(`${value}`) + ], []) } /** From dd78f51fbb352f7a6eb26aff51a3f15365caac68 Mon Sep 17 00:00:00 2001 From: KaKa Date: Wed, 8 Feb 2023 03:39:20 +0800 Subject: [PATCH 9/9] test: fix node@12 support --- test/issue-1903.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/issue-1903.js b/test/issue-1903.js index 42d8f2a3990..8478b2d53dd 100644 --- a/test/issue-1903.js +++ b/test/issue-1903.js @@ -1,6 +1,6 @@ 'use strict' -const { createServer } = require('node:http') +const { createServer } = require('http') const { test } = require('tap') const { request } = require('..')