Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw on invalid status codes #4212

Merged
merged 30 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
739bc47
check status code is integer, or string integer, in range
jonchurch Mar 7, 2020
1847262
fix tests, update jsdoc comment for res.status
jonchurch Mar 10, 2020
964f83a
throw if number is string
jonchurch Mar 10, 2020
9fa5241
narrow valid range to between 1xx and 5xx
jonchurch Apr 28, 2024
0761255
disambiguate the error message
jonchurch Apr 28, 2024
fa9d20f
Merge branch '5.x' into status-throw
jonchurch Apr 28, 2024
00ecf4d
update skipped tests, remove invalid string test
jonchurch Apr 28, 2024
a2a1448
remove invalid float test
jonchurch Apr 28, 2024
d0bd835
fixup! remove invalid float test
jonchurch Apr 28, 2024
0670a3b
fix invalid range tests error assertions
jonchurch Apr 28, 2024
8fc8c95
remove unused deprecate function
jonchurch Apr 28, 2024
063a3bb
add test to assert on 200.00 coming through as 200
jonchurch Apr 28, 2024
a6e4e9b
revert back to throwing only on > 999 and < 100
jonchurch May 4, 2024
cca1438
update implementation for > 999
jonchurch May 4, 2024
ce18152
add test for 700 status code
jonchurch May 4, 2024
bade7a1
update history with change
jonchurch May 4, 2024
eabe2cd
update jsdoc
jonchurch May 4, 2024
1a13582
clarify jsdoc comment
jonchurch May 4, 2024
8374bdb
one more round of jsdoc
jonchurch May 4, 2024
cee48e7
update 501 test
jonchurch May 4, 2024
5130879
add invalid status code test for res.sendStatus
jonchurch May 4, 2024
63192c4
add test describe block for valid range
jonchurch May 4, 2024
f7b7e35
fixup! add test describe block for valid range
jonchurch May 4, 2024
86eb68c
reduce the describe nesting
jonchurch May 4, 2024
6bc5136
switch to testing status 100, to avoid 100-continue behavior
jonchurch May 4, 2024
bbf58ba
fix 900 test
jonchurch May 4, 2024
64e8576
stringify code in thrown RangeError message
jonchurch Jun 5, 2024
4dbf773
remove accidentally duplicated res.status method
jonchurch Jun 5, 2024
bf4541b
fix error range message
jonchurch Jun 8, 2024
794f2de
update sendStatus invalid code test to use sendStatus
jonchurch Jul 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions History.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
unreleased
=========================
* breaking:
* `res.status()` accepts only integers, and input must be greater than 99 and less than 999
jonchurch marked this conversation as resolved.
Show resolved Hide resolved
* 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
=========================

Expand Down
28 changes: 19 additions & 9 deletions lib/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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)) {
jonchurch marked this conversation as resolved.
Show resolved Hide resolved
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;
};
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -847,7 +857,7 @@ res.redirect = function redirect(url) {
});

// Respond
this.statusCode = status;
this.status(status);
jonchurch marked this conversation as resolved.
Show resolved Hide resolved
this.set('Content-Length', Buffer.byteLength(body));

if (this.req.method === 'HEAD') {
Expand Down
12 changes: 12 additions & 0 deletions test/res.sendStatus.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.status(undefined).end()
jonchurch marked this conversation as resolved.
Show resolved Hide resolved
})

request(app)
.get('/')
.expect(500, /TypeError: Invalid status code/, done)
})
})
})
206 changes: 103 additions & 103 deletions test/res.status.js
Original file line number Diff line number Diff line change
@@ -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()

Expand All @@ -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()

Expand All @@ -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()

Expand All @@ -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()

Expand All @@ -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);
});
});
});
});

Loading