Skip to content

Commit 723b545

Browse files
jonchurchctcpip
jonchurch
and
ctcpip
authored
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 <[email protected]> * update sendStatus invalid code test to use sendStatus --------- Co-authored-by: Chris de Almeida <[email protected]>
1 parent ee40a88 commit 723b545

File tree

4 files changed

+141
-112
lines changed

4 files changed

+141
-112
lines changed

Diff for: ‎History.md

+7
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
unreleased
2+
=========================
3+
* breaking:
4+
* `res.status()` accepts only integers, and input must be greater than 99 and less than 1000
5+
* will throw a `RangeError: Invalid status code: ${code}. Status code must be greater than 99 and less than 1000.` for inputs outside this range
6+
* will throw a `TypeError: Invalid status code: ${code}. Status code must be an integer.` for non integer inputs
7+
18
5.0.0-beta.3 / 2024-03-25
29
=========================
310

Diff for: ‎lib/response.js

+19-9
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
var Buffer = require('safe-buffer').Buffer
1616
var contentDisposition = require('content-disposition');
1717
var createError = require('http-errors')
18-
var deprecate = require('depd')('express');
1918
var encodeUrl = require('encodeurl');
2019
var escapeHtml = require('escape-html');
2120
var http = require('http');
@@ -57,17 +56,28 @@ module.exports = res
5756
var schemaAndHostRegExp = /^(?:[a-zA-Z][a-zA-Z0-9+.-]*:)?\/\/[^\\\/\?]+/;
5857

5958
/**
60-
* Set status `code`.
59+
* Set the HTTP status code for the response.
6160
*
62-
* @param {Number} code
63-
* @return {ServerResponse}
61+
* Expects an integer value between 100 and 999 inclusive.
62+
* Throws an error if the provided status code is not an integer or if it's outside the allowable range.
63+
*
64+
* @param {number} code - The HTTP status code to set.
65+
* @return {ServerResponse} - Returns itself for chaining methods.
66+
* @throws {TypeError} If `code` is not an integer.
67+
* @throws {RangeError} If `code` is outside the range 100 to 999.
6468
* @public
6569
*/
6670

6771
res.status = function status(code) {
68-
if ((typeof code === 'string' || Math.floor(code) !== code) && code > 99 && code < 1000) {
69-
deprecate('res.status(' + JSON.stringify(code) + '): use res.status(' + Math.floor(code) + ') instead')
72+
// Check if the status code is not an integer
73+
if (!Number.isInteger(code)) {
74+
throw new TypeError(`Invalid status code: ${JSON.stringify(code)}. Status code must be an integer.`);
7075
}
76+
// Check if the status code is outside of Node's valid range
77+
if (code < 100 || code > 999) {
78+
throw new RangeError(`Invalid status code: ${JSON.stringify(code)}. Status code must be greater than 99 and less than 1000.`);
79+
}
80+
7181
this.statusCode = code;
7282
return this;
7383
};
@@ -182,7 +192,7 @@ res.send = function send(body) {
182192
}
183193

184194
// freshness
185-
if (req.fresh) this.statusCode = 304;
195+
if (req.fresh) this.status(304);
186196

187197
// strip irrelevant headers
188198
if (204 === this.statusCode || 304 === this.statusCode) {
@@ -314,7 +324,7 @@ res.jsonp = function jsonp(obj) {
314324
res.sendStatus = function sendStatus(statusCode) {
315325
var body = statuses.message[statusCode] || String(statusCode)
316326

317-
this.statusCode = statusCode;
327+
this.status(statusCode);
318328
this.type('txt');
319329

320330
return this.send(body);
@@ -847,7 +857,7 @@ res.redirect = function redirect(url) {
847857
});
848858

849859
// Respond
850-
this.statusCode = status;
860+
this.status(status);
851861
this.set('Content-Length', Buffer.byteLength(body));
852862

853863
if (this.req.method === 'HEAD') {

Diff for: ‎test/res.sendStatus.js

+12
Original file line numberDiff line numberDiff line change
@@ -28,5 +28,17 @@ describe('res', function () {
2828
.get('/')
2929
.expect(599, '599', done);
3030
})
31+
32+
it('should raise error for invalid status code', function (done) {
33+
var app = express()
34+
35+
app.use(function (req, res) {
36+
res.sendStatus(undefined).end()
37+
})
38+
39+
request(app)
40+
.get('/')
41+
.expect(500, /TypeError: Invalid status code/, done)
42+
})
3143
})
3244
})

Diff for: ‎test/res.status.js

+103-103
Original file line numberDiff line numberDiff line change
@@ -1,59 +1,36 @@
11
'use strict'
2-
3-
var express = require('../')
4-
var request = require('supertest')
5-
6-
var isIoJs = process.release
7-
? process.release.name === 'io.js'
8-
: ['v1.', 'v2.', 'v3.'].indexOf(process.version.slice(0, 3)) !== -1
2+
const express = require('../.');
3+
const request = require('supertest');
94

105
describe('res', function () {
116
describe('.status(code)', function () {
12-
// This test fails in node 4.0.0
13-
// https://github.com/expressjs/express/pull/2237/checks
14-
// As this will all be removed when https://github.com/expressjs/express/pull/4212
15-
// lands I am skipping for now and we can delete with that PR
16-
describe.skip('when "code" is undefined', function () {
17-
it('should raise error for invalid status code', function (done) {
18-
var app = express()
197

20-
app.use(function (req, res) {
21-
res.status(undefined).end()
22-
})
8+
it('should set the status code when valid', function (done) {
9+
var app = express();
2310

24-
request(app)
25-
.get('/')
26-
.expect(500, /Invalid status code/, function (err) {
27-
if (isIoJs) {
28-
done(err ? null : new Error('expected error'))
29-
} else {
30-
done(err)
31-
}
32-
})
33-
})
34-
})
11+
app.use(function (req, res) {
12+
res.status(200).end();
13+
});
3514

36-
describe.skip('when "code" is null', function () {
37-
it('should raise error for invalid status code', function (done) {
15+
request(app)
16+
.get('/')
17+
.expect(200, done);
18+
});
19+
20+
describe('accept valid ranges', function() {
21+
// not testing w/ 100, because that has specific meaning and behavior in Node as Expect: 100-continue
22+
it('should set the response status code to 101', function (done) {
3823
var app = express()
3924

4025
app.use(function (req, res) {
41-
res.status(null).end()
26+
res.status(101).end()
4227
})
4328

4429
request(app)
4530
.get('/')
46-
.expect(500, /Invalid status code/, function (err) {
47-
if (isIoJs) {
48-
done(err ? null : new Error('expected error'))
49-
} else {
50-
done(err)
51-
}
52-
})
31+
.expect(101, done)
5332
})
54-
})
5533

56-
describe('when "code" is 201', function () {
5734
it('should set the response status code to 201', function (done) {
5835
var app = express()
5936

@@ -65,9 +42,7 @@ describe('res', function () {
6542
.get('/')
6643
.expect(201, done)
6744
})
68-
})
6945

70-
describe('when "code" is 302', function () {
7146
it('should set the response status code to 302', function (done) {
7247
var app = express()
7348

@@ -79,9 +54,7 @@ describe('res', function () {
7954
.get('/')
8055
.expect(302, done)
8156
})
82-
})
8357

84-
describe('when "code" is 403', function () {
8558
it('should set the response status code to 403', function (done) {
8659
var app = express()
8760

@@ -93,9 +66,7 @@ describe('res', function () {
9366
.get('/')
9467
.expect(403, done)
9568
})
96-
})
9769

98-
describe('when "code" is 501', function () {
9970
it('should set the response status code to 501', function (done) {
10071
var app = express()
10172

@@ -107,100 +78,129 @@ describe('res', function () {
10778
.get('/')
10879
.expect(501, done)
10980
})
110-
})
11181

112-
describe('when "code" is "410"', function () {
113-
it('should set the response status code to 410', function (done) {
82+
it('should set the response status code to 700', function (done) {
11483
var app = express()
11584

11685
app.use(function (req, res) {
117-
res.status('410').end()
86+
res.status(700).end()
11887
})
11988

12089
request(app)
12190
.get('/')
122-
.expect(410, done)
91+
.expect(700, done)
12392
})
124-
})
12593

126-
describe.skip('when "code" is 410.1', function () {
127-
it('should set the response status code to 410', function (done) {
94+
it('should set the response status code to 800', function (done) {
12895
var app = express()
12996

13097
app.use(function (req, res) {
131-
res.status(410.1).end()
98+
res.status(800).end()
13299
})
133100

134101
request(app)
135102
.get('/')
136-
.expect(410, function (err) {
137-
if (isIoJs) {
138-
done(err ? null : new Error('expected error'))
139-
} else {
140-
done(err)
141-
}
142-
})
103+
.expect(800, done)
143104
})
144-
})
145105

146-
describe.skip('when "code" is 1000', function () {
147-
it('should raise error for invalid status code', function (done) {
106+
it('should set the response status code to 900', function (done) {
148107
var app = express()
149108

150109
app.use(function (req, res) {
151-
res.status(1000).end()
110+
res.status(900).end()
152111
})
153112

154113
request(app)
155114
.get('/')
156-
.expect(500, /Invalid status code/, function (err) {
157-
if (isIoJs) {
158-
done(err ? null : new Error('expected error'))
159-
} else {
160-
done(err)
161-
}
162-
})
115+
.expect(900, done)
163116
})
164117
})
165118

166-
describe.skip('when "code" is 99', function () {
167-
it('should raise error for invalid status code', function (done) {
168-
var app = express()
119+
describe('invalid status codes', function () {
120+
it('should raise error for status code below 100', function (done) {
121+
var app = express();
169122

170123
app.use(function (req, res) {
171-
res.status(99).end()
172-
})
124+
res.status(99).end();
125+
});
173126

174127
request(app)
175128
.get('/')
176-
.expect(500, /Invalid status code/, function (err) {
177-
if (isIoJs) {
178-
done(err ? null : new Error('expected error'))
179-
} else {
180-
done(err)
181-
}
182-
})
183-
})
184-
})
129+
.expect(500, /Invalid status code/, done);
130+
});
185131

186-
describe.skip('when "code" is -401', function () {
187-
it('should raise error for invalid status code', function (done) {
188-
var app = express()
132+
it('should raise error for status code above 999', function (done) {
133+
var app = express();
189134

190135
app.use(function (req, res) {
191-
res.status(-401).end()
192-
})
136+
res.status(1000).end();
137+
});
193138

194139
request(app)
195140
.get('/')
196-
.expect(500, /Invalid status code/, function (err) {
197-
if (isIoJs) {
198-
done(err ? null : new Error('expected error'))
199-
} else {
200-
done(err)
201-
}
202-
})
203-
})
204-
})
205-
})
206-
})
141+
.expect(500, /Invalid status code/, done);
142+
});
143+
144+
it('should raise error for non-integer status codes', function (done) {
145+
var app = express();
146+
147+
app.use(function (req, res) {
148+
res.status(200.1).end();
149+
});
150+
151+
request(app)
152+
.get('/')
153+
.expect(500, /Invalid status code/, done);
154+
});
155+
156+
it('should raise error for undefined status code', function (done) {
157+
var app = express();
158+
159+
app.use(function (req, res) {
160+
res.status(undefined).end();
161+
});
162+
163+
request(app)
164+
.get('/')
165+
.expect(500, /Invalid status code/, done);
166+
});
167+
168+
it('should raise error for null status code', function (done) {
169+
var app = express();
170+
171+
app.use(function (req, res) {
172+
res.status(null).end();
173+
});
174+
175+
request(app)
176+
.get('/')
177+
.expect(500, /Invalid status code/, done);
178+
});
179+
180+
it('should raise error for string status code', function (done) {
181+
var app = express();
182+
183+
app.use(function (req, res) {
184+
res.status("200").end();
185+
});
186+
187+
request(app)
188+
.get('/')
189+
.expect(500, /Invalid status code/, done);
190+
});
191+
192+
it('should raise error for NaN status code', function (done) {
193+
var app = express();
194+
195+
app.use(function (req, res) {
196+
res.status(NaN).end();
197+
});
198+
199+
request(app)
200+
.get('/')
201+
.expect(500, /Invalid status code/, done);
202+
});
203+
});
204+
});
205+
});
206+

0 commit comments

Comments
 (0)