From df327002c2384d187d20be0803ab2e2e516c1502 Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Tue, 2 Jun 2026 11:27:06 +0000 Subject: [PATCH 1/5] test/nginx: prevent request() from normalising paths This will be useful for testing nginx config at some point, and very misleading if trying to test path normalisation differences if request() collapses path traversals. --- test/nginx/src/mocha/nginx.spec.js | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/test/nginx/src/mocha/nginx.spec.js b/test/nginx/src/mocha/nginx.spec.js index 1cdb9ab28..065c2bf4f 100644 --- a/test/nginx/src/mocha/nginx.spec.js +++ b/test/nginx/src/mocha/nginx.spec.js @@ -1,3 +1,4 @@ +const { isIPv6 } = require('node:net'); const tls = require('node:tls'); const { Readable } = require('stream'); @@ -1087,13 +1088,20 @@ async function resetMock(port) { // // 1. do not follow redirects // 2. allow overriding of fetch's "forbidden" headers: https://developer.mozilla.org/en-US/docs/Glossary/Forbidden_header_name -function request(url, { body, ...options }={}) { +function request(urlString, { body, ...options }={}) { if(!options.headers) options.headers = {}; if(!options.headers.host) options.headers.host = 'odk-nginx.example.test'; + const url = new URL(urlString); + if(url.username || url.password) throw new Error('Basic auth creds not yet supported.'); + + options.host = safeIpv6(url.hostname); + options.port = url.port; + options.path = urlString.replace(/^http(s?):\/\/[^/]*/, '') || '/'; + return new Promise((resolve, reject) => { try { - const req = getProtocolImplFrom(url).request(url, options, res => { + const req = getProtocolImplFrom(url).request(options, res => { res.on('error', reject); const body = new Readable({ read:() => {} }); @@ -1129,8 +1137,7 @@ function request(url, { body, ...options }={}) { }); } -function getProtocolImplFrom(url) { - const { protocol } = new URL(url); +function getProtocolImplFrom({ protocol }) { switch(protocol) { case 'http:': return require('node:http'); case 'https:': return require('node:https'); @@ -1194,3 +1201,8 @@ function assertCsp(actual, expected) { ); } } + +function safeIpv6(hostname) { + const maybeV6 = hostname.replace(/^\[(.*)\]$/, (_, $1) => $1); + return isIPv6(maybeV6) ? maybeV6 : hostname; +} From 1a2c78ed1188c828d854c0419b4c72e6b4eda4cf Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Sun, 7 Jun 2026 06:28:32 +0000 Subject: [PATCH 2/5] add test, fix bug --- test/nginx/src/mocha/request.js | 2 +- test/nginx/src/mocha/request.spec.js | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/test/nginx/src/mocha/request.js b/test/nginx/src/mocha/request.js index 2226f1470..33e7d7c40 100644 --- a/test/nginx/src/mocha/request.js +++ b/test/nginx/src/mocha/request.js @@ -20,7 +20,7 @@ function request(urlString, { body, ...options }={}) { return new Promise((resolve, reject) => { try { - const req = getProtocolImplFrom(url).request(url, options, res => { + const req = getProtocolImplFrom(url).request(options, res => { res.on('error', reject); const body = new Readable({ read:() => {} }); diff --git a/test/nginx/src/mocha/request.spec.js b/test/nginx/src/mocha/request.spec.js index cc4929af1..3ba319e40 100644 --- a/test/nginx/src/mocha/request.spec.js +++ b/test/nginx/src/mocha/request.spec.js @@ -15,7 +15,7 @@ describe('request()', () => { const app = express(); app.use((req, res, next) => { - const { method, path, headers } = req; + const { method, originalUrl:path, headers } = req; requestsReceived.push({ method, path, headers }); next(); }); @@ -66,6 +66,17 @@ describe('request()', () => { ]); assert.equal(requestsReceived[0].headers['host'], 'not-a-host'); }); + + it('should not normalise URLs', async () => { + // when + const res = await request(`http://127.0.0.1:${port}/a/../b.html?x=1`); + + // then + assert.equal(res.status, 200); + assert.deepEqual(stripHeaders(requestsReceived), [ + { method:'GET', path:'/a/../b.html?x=1' }, + ]); + }); }); function stripHeaders(arr) { From 93a4f7d4e580db7214037a594825500a6a69fb99 Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Sun, 7 Jun 2026 11:43:11 +0000 Subject: [PATCH 3/5] document better --- test/nginx/src/mocha/request.js | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/test/nginx/src/mocha/request.js b/test/nginx/src/mocha/request.js index 33e7d7c40..23f37e1d1 100644 --- a/test/nginx/src/mocha/request.js +++ b/test/nginx/src/mocha/request.js @@ -7,20 +7,13 @@ module.exports = request; // // 1. do not follow redirects // 2. allow overriding of fetch's "forbidden" headers: https://developer.mozilla.org/en-US/docs/Glossary/Forbidden_header_name -function request(urlString, { body, ...options }={}) { +function request(url, { body, ...options }={}) { if(!options.headers) options.headers = {}; if(!options.headers.host) options.headers.host = 'odk-nginx.example.test'; - const url = new URL(urlString); - if(url.username || url.password) throw new Error('Basic auth creds not yet supported.'); - - options.host = safeIpv6(url.hostname); - options.port = url.port; - options.path = urlString.replace(/^http(s?):\/\/[^/]*/, '') || '/'; - return new Promise((resolve, reject) => { try { - const req = getProtocolImplFrom(url).request(options, res => { + const req = getProtocolImplFrom(url).request({ ...options, ...preserve(url) }, res => { res.on('error', reject); const body = new Readable({ read:() => {} }); @@ -56,7 +49,8 @@ function request(urlString, { body, ...options }={}) { }); } -function getProtocolImplFrom({ protocol }) { +function getProtocolImplFrom(url) { + const { protocol } = new URL(url); switch(protocol) { case 'http:': return require('node:http'); case 'https:': return require('node:https'); @@ -64,6 +58,23 @@ function getProtocolImplFrom({ protocol }) { } } +/** + * Prevent URL path normalisation. + * + * @see https://nodejs.org/api/http.html#httprequesturl-options-callback + * @see https://nodejs.org/api/url.html#new-urlinput-base + */ +function preserve(urlString) { + const url = new URL(urlString); + if(url.username || url.password) throw new Error('Basic auth creds not yet supported.'); + + const host = safeIpv6(url.hostname); + const port = url.port; + const path = urlString.replace(/^http(s?):\/\/[^/]*/, '') || '/'; + + return { host, port, path }; +} + function safeIpv6(hostname) { const maybeV6 = hostname.replace(/^\[(.*)\]$/, (_, $1) => $1); return isIPv6(maybeV6) ? maybeV6 : hostname; From d85e1350be0f57dc9f34e0edb8e104c8cdbe1a8f Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Sun, 7 Jun 2026 11:44:12 +0000 Subject: [PATCH 4/5] remove blank line --- test/nginx/src/mocha/request.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/nginx/src/mocha/request.js b/test/nginx/src/mocha/request.js index 23f37e1d1..509a8eff3 100644 --- a/test/nginx/src/mocha/request.js +++ b/test/nginx/src/mocha/request.js @@ -60,7 +60,6 @@ function getProtocolImplFrom(url) { /** * Prevent URL path normalisation. - * * @see https://nodejs.org/api/http.html#httprequesturl-options-callback * @see https://nodejs.org/api/url.html#new-urlinput-base */ From 1c5d6e600d28c0e5f529b5b7e610d3d72cc0fd60 Mon Sep 17 00:00:00 2001 From: alxndrsn Date: Sun, 7 Jun 2026 11:46:15 +0000 Subject: [PATCH 5/5] rename target --- test/nginx/src/mocha/request.spec.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/nginx/src/mocha/request.spec.js b/test/nginx/src/mocha/request.spec.js index 3ba319e40..bcb0a075c 100644 --- a/test/nginx/src/mocha/request.spec.js +++ b/test/nginx/src/mocha/request.spec.js @@ -15,8 +15,8 @@ describe('request()', () => { const app = express(); app.use((req, res, next) => { - const { method, originalUrl:path, headers } = req; - requestsReceived.push({ method, path, headers }); + const { method, originalUrl:target, headers } = req; + requestsReceived.push({ method, target, headers }); next(); }); app.get('/redirect-302', (req, res) => { @@ -46,7 +46,7 @@ describe('request()', () => { assert.equal(res.status, 302); assert.equal(res.headers.get('location'), 'http://example.test/redirected'); assert.deepEqual(stripHeaders(requestsReceived), [ - { method:'GET', path:'/redirect-302' }, + { method:'GET', target:'/redirect-302' }, ]); }); @@ -62,7 +62,7 @@ describe('request()', () => { // then assert.equal(res.status, 200); assert.deepEqual(stripHeaders(requestsReceived), [ - { method:'GET', path:'/' }, + { method:'GET', target:'/' }, ]); assert.equal(requestsReceived[0].headers['host'], 'not-a-host'); }); @@ -74,7 +74,7 @@ describe('request()', () => { // then assert.equal(res.status, 200); assert.deepEqual(stripHeaders(requestsReceived), [ - { method:'GET', path:'/a/../b.html?x=1' }, + { method:'GET', target:'/a/../b.html?x=1' }, ]); }); });