From 723b5451bbcbab69bc8ded50ccd6545a79b8fe64 Mon Sep 17 00:00:00 2001 From: Jon Church Date: Tue, 30 Jul 2024 17:49:13 -0400 Subject: [PATCH 1/5] Throw on invalid status codes (#4212) * check status code is integer, or string integer, in range * fix tests, update jsdoc comment for res.status * throw if number is string * narrow valid range to between 1xx and 5xx * disambiguate the error message * update skipped tests, remove invalid string test * remove invalid float test * fixup! remove invalid float test * fix invalid range tests error assertions * remove unused deprecate function * add test to assert on 200.00 coming through as 200 this is the behavior of node's underlying HTTP module * revert back to throwing only on > 999 and < 100 * update implementation for > 999 * add test for 700 status code * update history with change * update jsdoc * clarify jsdoc comment * one more round of jsdoc * update 501 test * add invalid status code test for res.sendStatus * add test describe block for valid range * fixup! add test describe block for valid range * reduce the describe nesting * switch to testing status 100, to avoid 100-continue behavior * fix 900 test * stringify code in thrown RangeError message * remove accidentally duplicated res.status method * fix error range message Co-authored-by: Chris de Almeida * update sendStatus invalid code test to use sendStatus --------- Co-authored-by: Chris de Almeida --- History.md | 7 ++ lib/response.js | 28 ++++-- test/res.sendStatus.js | 12 +++ test/res.status.js | 206 ++++++++++++++++++++--------------------- 4 files changed, 141 insertions(+), 112 deletions(-) diff --git a/History.md b/History.md index 73fc46b26fb..89d5af3cebe 100644 --- a/History.md +++ b/History.md @@ -1,3 +1,10 @@ +unreleased +========================= +* breaking: + * `res.status()` accepts only integers, and input must be greater than 99 and less than 1000 + * will throw a `RangeError: Invalid status code: ${code}. Status code must be greater than 99 and less than 1000.` for inputs outside this range + * will throw a `TypeError: Invalid status code: ${code}. Status code must be an integer.` for non integer inputs + 5.0.0-beta.3 / 2024-03-25 ========================= diff --git a/lib/response.js b/lib/response.js index 14743817a96..6ad54dbfc7e 100644 --- a/lib/response.js +++ b/lib/response.js @@ -15,7 +15,6 @@ var Buffer = require('safe-buffer').Buffer var contentDisposition = require('content-disposition'); var createError = require('http-errors') -var deprecate = require('depd')('express'); var encodeUrl = require('encodeurl'); var escapeHtml = require('escape-html'); var http = require('http'); @@ -57,17 +56,28 @@ module.exports = res var schemaAndHostRegExp = /^(?:[a-zA-Z][a-zA-Z0-9+.-]*:)?\/\/[^\\\/\?]+/; /** - * Set status `code`. + * Set the HTTP status code for the response. * - * @param {Number} code - * @return {ServerResponse} + * Expects an integer value between 100 and 999 inclusive. + * Throws an error if the provided status code is not an integer or if it's outside the allowable range. + * + * @param {number} code - The HTTP status code to set. + * @return {ServerResponse} - Returns itself for chaining methods. + * @throws {TypeError} If `code` is not an integer. + * @throws {RangeError} If `code` is outside the range 100 to 999. * @public */ res.status = function status(code) { - if ((typeof code === 'string' || Math.floor(code) !== code) && code > 99 && code < 1000) { - deprecate('res.status(' + JSON.stringify(code) + '): use res.status(' + Math.floor(code) + ') instead') + // Check if the status code is not an integer + if (!Number.isInteger(code)) { + throw new TypeError(`Invalid status code: ${JSON.stringify(code)}. Status code must be an integer.`); } + // Check if the status code is outside of Node's valid range + if (code < 100 || code > 999) { + throw new RangeError(`Invalid status code: ${JSON.stringify(code)}. Status code must be greater than 99 and less than 1000.`); + } + this.statusCode = code; return this; }; @@ -182,7 +192,7 @@ res.send = function send(body) { } // freshness - if (req.fresh) this.statusCode = 304; + if (req.fresh) this.status(304); // strip irrelevant headers if (204 === this.statusCode || 304 === this.statusCode) { @@ -314,7 +324,7 @@ res.jsonp = function jsonp(obj) { res.sendStatus = function sendStatus(statusCode) { var body = statuses.message[statusCode] || String(statusCode) - this.statusCode = statusCode; + this.status(statusCode); this.type('txt'); return this.send(body); @@ -847,7 +857,7 @@ res.redirect = function redirect(url) { }); // Respond - this.statusCode = status; + this.status(status); this.set('Content-Length', Buffer.byteLength(body)); if (this.req.method === 'HEAD') { diff --git a/test/res.sendStatus.js b/test/res.sendStatus.js index 9b1de8385cd..b244cf9d173 100644 --- a/test/res.sendStatus.js +++ b/test/res.sendStatus.js @@ -28,5 +28,17 @@ describe('res', function () { .get('/') .expect(599, '599', done); }) + + it('should raise error for invalid status code', function (done) { + var app = express() + + app.use(function (req, res) { + res.sendStatus(undefined).end() + }) + + request(app) + .get('/') + .expect(500, /TypeError: Invalid status code/, done) + }) }) }) diff --git a/test/res.status.js b/test/res.status.js index d2fc6a28c13..59c8a57e702 100644 --- a/test/res.status.js +++ b/test/res.status.js @@ -1,59 +1,36 @@ 'use strict' - -var express = require('../') -var request = require('supertest') - -var isIoJs = process.release - ? process.release.name === 'io.js' - : ['v1.', 'v2.', 'v3.'].indexOf(process.version.slice(0, 3)) !== -1 +const express = require('../.'); +const request = require('supertest'); describe('res', function () { describe('.status(code)', function () { - // This test fails in node 4.0.0 - // https://github.com/expressjs/express/pull/2237/checks - // As this will all be removed when https://github.com/expressjs/express/pull/4212 - // lands I am skipping for now and we can delete with that PR - describe.skip('when "code" is undefined', function () { - it('should raise error for invalid status code', function (done) { - var app = express() - app.use(function (req, res) { - res.status(undefined).end() - }) + it('should set the status code when valid', function (done) { + var app = express(); - request(app) - .get('/') - .expect(500, /Invalid status code/, function (err) { - if (isIoJs) { - done(err ? null : new Error('expected error')) - } else { - done(err) - } - }) - }) - }) + app.use(function (req, res) { + res.status(200).end(); + }); - describe.skip('when "code" is null', function () { - it('should raise error for invalid status code', function (done) { + request(app) + .get('/') + .expect(200, done); + }); + + describe('accept valid ranges', function() { + // not testing w/ 100, because that has specific meaning and behavior in Node as Expect: 100-continue + it('should set the response status code to 101', function (done) { var app = express() app.use(function (req, res) { - res.status(null).end() + res.status(101).end() }) request(app) .get('/') - .expect(500, /Invalid status code/, function (err) { - if (isIoJs) { - done(err ? null : new Error('expected error')) - } else { - done(err) - } - }) + .expect(101, done) }) - }) - describe('when "code" is 201', function () { it('should set the response status code to 201', function (done) { var app = express() @@ -65,9 +42,7 @@ describe('res', function () { .get('/') .expect(201, done) }) - }) - describe('when "code" is 302', function () { it('should set the response status code to 302', function (done) { var app = express() @@ -79,9 +54,7 @@ describe('res', function () { .get('/') .expect(302, done) }) - }) - describe('when "code" is 403', function () { it('should set the response status code to 403', function (done) { var app = express() @@ -93,9 +66,7 @@ describe('res', function () { .get('/') .expect(403, done) }) - }) - describe('when "code" is 501', function () { it('should set the response status code to 501', function (done) { var app = express() @@ -107,100 +78,129 @@ describe('res', function () { .get('/') .expect(501, done) }) - }) - describe('when "code" is "410"', function () { - it('should set the response status code to 410', function (done) { + it('should set the response status code to 700', function (done) { var app = express() app.use(function (req, res) { - res.status('410').end() + res.status(700).end() }) request(app) .get('/') - .expect(410, done) + .expect(700, done) }) - }) - describe.skip('when "code" is 410.1', function () { - it('should set the response status code to 410', function (done) { + it('should set the response status code to 800', function (done) { var app = express() app.use(function (req, res) { - res.status(410.1).end() + res.status(800).end() }) request(app) .get('/') - .expect(410, function (err) { - if (isIoJs) { - done(err ? null : new Error('expected error')) - } else { - done(err) - } - }) + .expect(800, done) }) - }) - describe.skip('when "code" is 1000', function () { - it('should raise error for invalid status code', function (done) { + it('should set the response status code to 900', function (done) { var app = express() app.use(function (req, res) { - res.status(1000).end() + res.status(900).end() }) request(app) .get('/') - .expect(500, /Invalid status code/, function (err) { - if (isIoJs) { - done(err ? null : new Error('expected error')) - } else { - done(err) - } - }) + .expect(900, done) }) }) - describe.skip('when "code" is 99', function () { - it('should raise error for invalid status code', function (done) { - var app = express() + describe('invalid status codes', function () { + it('should raise error for status code below 100', function (done) { + var app = express(); app.use(function (req, res) { - res.status(99).end() - }) + res.status(99).end(); + }); request(app) .get('/') - .expect(500, /Invalid status code/, function (err) { - if (isIoJs) { - done(err ? null : new Error('expected error')) - } else { - done(err) - } - }) - }) - }) + .expect(500, /Invalid status code/, done); + }); - describe.skip('when "code" is -401', function () { - it('should raise error for invalid status code', function (done) { - var app = express() + it('should raise error for status code above 999', function (done) { + var app = express(); app.use(function (req, res) { - res.status(-401).end() - }) + res.status(1000).end(); + }); request(app) .get('/') - .expect(500, /Invalid status code/, function (err) { - if (isIoJs) { - done(err ? null : new Error('expected error')) - } else { - done(err) - } - }) - }) - }) - }) -}) + .expect(500, /Invalid status code/, done); + }); + + it('should raise error for non-integer status codes', function (done) { + var app = express(); + + app.use(function (req, res) { + res.status(200.1).end(); + }); + + request(app) + .get('/') + .expect(500, /Invalid status code/, done); + }); + + it('should raise error for undefined status code', function (done) { + var app = express(); + + app.use(function (req, res) { + res.status(undefined).end(); + }); + + request(app) + .get('/') + .expect(500, /Invalid status code/, done); + }); + + it('should raise error for null status code', function (done) { + var app = express(); + + app.use(function (req, res) { + res.status(null).end(); + }); + + request(app) + .get('/') + .expect(500, /Invalid status code/, done); + }); + + it('should raise error for string status code', function (done) { + var app = express(); + + app.use(function (req, res) { + res.status("200").end(); + }); + + request(app) + .get('/') + .expect(500, /Invalid status code/, done); + }); + + it('should raise error for NaN status code', function (done) { + var app = express(); + + app.use(function (req, res) { + res.status(NaN).end(); + }); + + request(app) + .get('/') + .expect(500, /Invalid status code/, done); + }); + }); + }); +}); + From d106bf53246eb82aeb925c84c8855c54e17741c2 Mon Sep 17 00:00:00 2001 From: "Mick A." Date: Thu, 1 Aug 2024 17:42:07 -0600 Subject: [PATCH 2/5] Use Array.flat instead of array-flatten (#5677) --- lib/application.js | 4 ++-- package.json | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/application.js b/lib/application.js index 43c9f34b062..ecfe2186db7 100644 --- a/lib/application.js +++ b/lib/application.js @@ -21,7 +21,6 @@ var http = require('http'); var compileETag = require('./utils').compileETag; var compileQueryParser = require('./utils').compileQueryParser; var compileTrust = require('./utils').compileTrust; -var flatten = require('array-flatten').flatten var merge = require('utils-merge'); var resolve = require('path').resolve; var once = require('once') @@ -34,6 +33,7 @@ var setPrototypeOf = require('setprototypeof') */ var slice = Array.prototype.slice; +var flatten = Array.prototype.flat; /** * Application prototype. @@ -209,7 +209,7 @@ app.use = function use(fn) { } } - var fns = flatten(slice.call(arguments, offset)); + var fns = flatten.call(slice.call(arguments, offset), Infinity); if (fns.length === 0) { throw new TypeError('app.use() requires a middleware function') diff --git a/package.json b/package.json index d3e2f0a1909..5ec37a8f773 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,6 @@ ], "dependencies": { "accepts": "~1.3.8", - "array-flatten": "3.0.0", "body-parser": "2.0.0-beta.2", "content-disposition": "0.5.4", "content-type": "~1.0.4", From 160b91cbf79b595712b694c3513c347551d17fbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulises=20Gasc=C3=B3n?= Date: Fri, 2 Aug 2024 16:07:36 +0200 Subject: [PATCH 3/5] feat: adopt Node@18 as the minimum supported version (#5803) - PR-URL: https://github.com/expressjs/express/pull/5803 - This is a BREAKING CHANGE --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 5ec37a8f773..37b0a63ed4b 100644 --- a/package.json +++ b/package.json @@ -81,7 +81,7 @@ "vhost": "~3.0.2" }, "engines": { - "node": ">= 4" + "node": ">= 18" }, "files": [ "LICENSE", From 82fc12a40b3e6694e9a2c9b1376e7548d95779f6 Mon Sep 17 00:00:00 2001 From: Jon Church Date: Fri, 2 Aug 2024 16:26:45 -0400 Subject: [PATCH 4/5] Ignore `expires` and `maxAge` in `res.clearCookie()` (#5792) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * add test for removing user provided expires * rework impl and tests to ignore maxAge, do not set it this is to take into account the built-in relative expires when passing a maxAge to res.cookie I realized that using maxAge to invalidate cookies inherrently hit this relativee expires behavior, and the goal of this PR is not to rework that relative expires behavior w/ maxAge, but to prevent users from overwriting these values by accident when clearing cookies * update history.md * explicitly delete maxAge instead of setting as undefined * drop the spread, use object.assign * wording, review comment on history.md Co-authored-by: Chris de Almeida * ♻️ use spread, update supported ecmascript version --------- Co-authored-by: Chris de Almeida --- .eslintrc.yml | 2 +- History.md | 2 ++ lib/response.js | 5 ++++- test/res.clearCookie.js | 26 ++++++++++++++++++++++++++ 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/.eslintrc.yml b/.eslintrc.yml index 9e282530d50..70bc9a6e7e1 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -1,6 +1,6 @@ root: true env: - es6: true + es2022: true node: true rules: eol-last: error diff --git a/History.md b/History.md index 89d5af3cebe..7c51a32d8be 100644 --- a/History.md +++ b/History.md @@ -4,6 +4,8 @@ unreleased * `res.status()` accepts only integers, and input must be greater than 99 and less than 1000 * will throw a `RangeError: Invalid status code: ${code}. Status code must be greater than 99 and less than 1000.` for inputs outside this range * will throw a `TypeError: Invalid status code: ${code}. Status code must be an integer.` for non integer inputs +* change: + - `res.clearCookie` will ignore user provided `maxAge` and `expires` options 5.0.0-beta.3 / 2024-03-25 ========================= diff --git a/lib/response.js b/lib/response.js index 6ad54dbfc7e..a5a33e86097 100644 --- a/lib/response.js +++ b/lib/response.js @@ -707,7 +707,10 @@ res.get = function(field){ */ res.clearCookie = function clearCookie(name, options) { - var opts = merge({ expires: new Date(1), path: '/' }, options); + // Force cookie expiration by setting expires to the past + const opts = { path: '/', ...options, expires: new Date(1)}; + // ensure maxAge is not passed + delete opts.maxAge return this.cookie(name, '', opts); }; diff --git a/test/res.clearCookie.js b/test/res.clearCookie.js index fc0cfb99a3d..74a746eb7be 100644 --- a/test/res.clearCookie.js +++ b/test/res.clearCookie.js @@ -32,5 +32,31 @@ describe('res', function(){ .expect('Set-Cookie', 'sid=; Path=/admin; Expires=Thu, 01 Jan 1970 00:00:00 GMT') .expect(200, done) }) + + it('should ignore maxAge', function(done){ + var app = express(); + + app.use(function(req, res){ + res.clearCookie('sid', { path: '/admin', maxAge: 1000 }).end(); + }); + + request(app) + .get('/') + .expect('Set-Cookie', 'sid=; Path=/admin; Expires=Thu, 01 Jan 1970 00:00:00 GMT') + .expect(200, done) + }) + + it('should ignore user supplied expires param', function(done){ + var app = express(); + + app.use(function(req, res){ + res.clearCookie('sid', { path: '/admin', expires: new Date() }).end(); + }); + + request(app) + .get('/') + .expect('Set-Cookie', 'sid=; Path=/admin; Expires=Thu, 01 Jan 1970 00:00:00 GMT') + .expect(200, done) + }) }) }) From b76130d34bf0745643266294fe33fb7825b925c4 Mon Sep 17 00:00:00 2001 From: Wes Todd Date: Thu, 25 Jul 2024 16:33:34 -0700 Subject: [PATCH 5/5] fix(deps)!: send@^1.0.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 37b0a63ed4b..834fa595796 100644 --- a/package.json +++ b/package.json @@ -54,7 +54,7 @@ "range-parser": "~1.2.1", "router": "2.0.0-beta.2", "safe-buffer": "5.2.1", - "send": "1.0.0-beta.2", + "send": "^1.0.0", "serve-static": "2.0.0-beta.2", "setprototypeof": "1.2.0", "statuses": "2.0.1",