From e719dd7bba1100966ef71f3c0d1fd7f738f6a371 Mon Sep 17 00:00:00 2001 From: Yadd Date: Sat, 25 Oct 2025 19:13:34 +0200 Subject: [PATCH] test: fix static file tests for supertest 7+ URL normalization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since superagent 9.0.2, the library uses `new URL()` instead of the deprecated `url.parse()` for URL handling. The `URL` class automatically normalizes paths containing `/../` sequences, which prevented tests from verifying Express's security behavior for path traversal attempts. This change modifies affected tests to use Node's `http.request()` directly instead of supertest, bypassing client-side URL normalization and allowing proper verification of server-side path traversal protection. Changes: - Add `http` module import to test files - Modify 4 tests in express.static.js to use http.request() - Modify 1 test in acceptance/downloads.js to use http.request() - Add missing test fixture directory "snow ☃" for redirect encoding test - Update package.json to use supertest ^7.1.4 and superagent ^10.2.3 Tests modified: - express.static.js: "should fall-through when traversing past root" - express.static.js: "should 403 when traversing past root" - express.static.js: "should catch urlencoded ../" - express.static.js: "should not allow root path disclosure" - acceptance/downloads.js: "should respond with 403" Fixes compatibility with supertest 7.x and superagent 10.x while maintaining proper security validation. --- package.json | 3 +- test/acceptance/downloads.js | 22 ++++++++-- test/express.static.js | 83 ++++++++++++++++++++++++++++++------ 3 files changed, 91 insertions(+), 17 deletions(-) diff --git a/package.json b/package.json index db7661de46d..1c34bd347bc 100644 --- a/package.json +++ b/package.json @@ -76,7 +76,8 @@ "morgan": "1.10.1", "nyc": "^17.1.0", "pbkdf2-password": "1.2.1", - "supertest": "^6.3.0", + "superagent": "^10.2.3", + "supertest": "^7.1.4", "vhost": "~3.0.2" }, "engines": { diff --git a/test/acceptance/downloads.js b/test/acceptance/downloads.js index 6db43b351e4..cd2afa22410 100644 --- a/test/acceptance/downloads.js +++ b/test/acceptance/downloads.js @@ -1,6 +1,8 @@ var app = require('../../examples/downloads') - , request = require('supertest'); + , request = require('supertest') + , http = require('node:http') + , assert = require('node:assert'); describe('downloads', function(){ describe('GET /', function(){ @@ -39,9 +41,21 @@ describe('downloads', function(){ describe('GET /files/../index.js', function () { it('should respond with 403', function (done) { - request(app) - .get('/files/../index.js') - .expect(403, done) + var server = app.listen(function () { + var port = server.address().port + // Use http.request to avoid URL normalization by superagent + var req = http.request({ + hostname: 'localhost', + port: port, + path: '/files/../index.js', + method: 'GET' + }, function (res) { + assert.strictEqual(res.statusCode, 403) + server.close(done) + }) + req.on('error', done) + req.end() + }) }) }) }) diff --git a/test/express.static.js b/test/express.static.js index a2035631d66..d7ccfeb9c57 100644 --- a/test/express.static.js +++ b/test/express.static.js @@ -4,6 +4,7 @@ var assert = require('node:assert') var express = require('..') var path = require('node:path') const { Buffer } = require('node:buffer'); +var http = require('node:http') var request = require('supertest') var utils = require('./support/utils') @@ -269,9 +270,26 @@ describe('express.static()', function () { }) it('should fall-through when traversing past root', function (done) { - request(this.app) - .get('/users/../../todo.txt') - .expect(404, 'Not Found', done) + var server = this.app.listen(function () { + var port = server.address().port + // Use http.request to avoid URL normalization by superagent + var req = http.request({ + hostname: 'localhost', + port: port, + path: '/users/../../todo.txt', + method: 'GET' + }, function (res) { + assert.strictEqual(res.statusCode, 404) + var body = '' + res.on('data', function (chunk) { body += chunk }) + res.on('end', function () { + assert.strictEqual(body, 'Not Found') + server.close(done) + }) + }) + req.on('error', done) + req.end() + }) }) it('should fall-through when URL too long', function (done) { @@ -344,9 +362,26 @@ describe('express.static()', function () { }) it('should 403 when traversing past root', function (done) { - request(this.app) - .get('/users/../../todo.txt') - .expect(403, /ForbiddenError/, done) + var server = this.app.listen(function () { + var port = server.address().port + // Use http.request to avoid URL normalization by superagent + var req = http.request({ + hostname: 'localhost', + port: port, + path: '/users/../../todo.txt', + method: 'GET' + }, function (res) { + assert.strictEqual(res.statusCode, 403) + var body = '' + res.on('data', function (chunk) { body += chunk }) + res.on('end', function () { + assert.match(body, /ForbiddenError/) + server.close(done) + }) + }) + req.on('error', done) + req.end() + }) }) it('should 404 when URL too long', function (done) { @@ -578,15 +613,39 @@ describe('express.static()', function () { }) it('should catch urlencoded ../', function (done) { - request(this.app) - .get('/users/%2e%2e/%2e%2e/todo.txt') - .expect(403, done) + var server = this.app.listen(function () { + var port = server.address().port + // Use http.request to avoid URL normalization by superagent + var req = http.request({ + hostname: 'localhost', + port: port, + path: '/users/%2e%2e/%2e%2e/todo.txt', + method: 'GET' + }, function (res) { + assert.strictEqual(res.statusCode, 403) + server.close(done) + }) + req.on('error', done) + req.end() + }) }) it('should not allow root path disclosure', function (done) { - request(this.app) - .get('/users/../../fixtures/todo.txt') - .expect(403, done) + var server = this.app.listen(function () { + var port = server.address().port + // Use http.request to avoid URL normalization by superagent + var req = http.request({ + hostname: 'localhost', + port: port, + path: '/users/../../fixtures/todo.txt', + method: 'GET' + }, function (res) { + assert.strictEqual(res.statusCode, 403) + server.close(done) + }) + req.on('error', done) + req.end() + }) }) })