Skip to content

Commit e6f1b1f

Browse files
committed
fix: throw TypeError for invalid url in res.redirect
Fixes #6941 Previously, res.redirect(undefined) would send an invalid Location: undefined header. This change throws a TypeError when the url argument is undefined or not a string, aligning with the behavior of other Express methods like res.sendFile. Changes: - Throw TypeError when url is undefined - Throw TypeError when url is not a string - Add comprehensive tests for invalid url handling
1 parent dbac741 commit e6f1b1f

File tree

2 files changed

+210
-3
lines changed

2 files changed

+210
-3
lines changed

lib/response.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -827,12 +827,13 @@ res.redirect = function redirect(url) {
827827
address = arguments[1]
828828
}
829829

830-
if (!address) {
831-
deprecate('Provide a url argument');
830+
// Validate URL argument
831+
if (address === undefined) {
832+
throw new TypeError('url argument is required to res.redirect');
832833
}
833834

834835
if (typeof address !== 'string') {
835-
deprecate('Url must be a string');
836+
throw new TypeError('url argument must be a string to res.redirect');
836837
}
837838

838839
if (typeof status !== 'number') {

test/res.redirect.issue-6941.js

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
'use strict'
2+
3+
var express = require('..');
4+
var request = require('supertest');
5+
6+
describe('res.redirect - Issue #6941', function () {
7+
describe('when url is undefined', function () {
8+
it('should throw a TypeError', function (done) {
9+
var app = express();
10+
11+
app.use(function (req, res) {
12+
res.redirect(undefined);
13+
});
14+
15+
app.use(function (err, req, res, next) {
16+
res.status(500).json({
17+
error: err.message,
18+
type: err.name
19+
});
20+
});
21+
22+
request(app)
23+
.get('/')
24+
.expect(500)
25+
.expect(function (res) {
26+
if (res.body.type !== 'TypeError') {
27+
throw new Error('Expected TypeError, got ' + res.body.type);
28+
}
29+
if (res.body.error !== 'url argument is required to res.redirect') {
30+
throw new Error('Unexpected error message: ' + res.body.error);
31+
}
32+
})
33+
.end(done);
34+
});
35+
36+
it('should not send Location: undefined header', function (done) {
37+
var app = express();
38+
39+
app.use(function (req, res) {
40+
res.redirect(undefined);
41+
});
42+
43+
app.use(function (err, req, res, next) {
44+
res.status(500).send('error');
45+
});
46+
47+
request(app)
48+
.get('/')
49+
.expect(500)
50+
.expect(function (res) {
51+
if (res.headers.location === 'undefined') {
52+
throw new Error('Location header should not be "undefined"');
53+
}
54+
})
55+
.end(done);
56+
});
57+
});
58+
59+
describe('when url is not a string', function () {
60+
it('should throw a TypeError for null', function (done) {
61+
var app = express();
62+
63+
app.use(function (req, res) {
64+
res.redirect(null);
65+
});
66+
67+
app.use(function (err, req, res, next) {
68+
res.status(500).json({
69+
error: err.message,
70+
type: err.name
71+
});
72+
});
73+
74+
request(app)
75+
.get('/')
76+
.expect(500)
77+
.expect(function (res) {
78+
if (res.body.type !== 'TypeError') {
79+
throw new Error('Expected TypeError, got ' + res.body.type);
80+
}
81+
})
82+
.end(done);
83+
});
84+
85+
it('should throw a TypeError for number (when single argument)', function (done) {
86+
var app = express();
87+
88+
app.use(function (req, res) {
89+
res.redirect(123);
90+
});
91+
92+
app.use(function (err, req, res, next) {
93+
res.status(500).json({
94+
error: err.message,
95+
type: err.name
96+
});
97+
});
98+
99+
request(app)
100+
.get('/')
101+
.expect(500)
102+
.expect(function (res) {
103+
if (res.body.type !== 'TypeError') {
104+
throw new Error('Expected TypeError, got ' + res.body.type);
105+
}
106+
})
107+
.end(done);
108+
});
109+
110+
it('should throw a TypeError for object', function (done) {
111+
var app = express();
112+
113+
app.use(function (req, res) {
114+
res.redirect({ url: '/home' });
115+
});
116+
117+
app.use(function (err, req, res, next) {
118+
res.status(500).json({
119+
error: err.message,
120+
type: err.name
121+
});
122+
});
123+
124+
request(app)
125+
.get('/')
126+
.expect(500)
127+
.expect(function (res) {
128+
if (res.body.type !== 'TypeError') {
129+
throw new Error('Expected TypeError, got ' + res.body.type);
130+
}
131+
})
132+
.end(done);
133+
});
134+
});
135+
136+
describe('when status and url are provided', function () {
137+
it('should throw a TypeError if url is undefined', function (done) {
138+
var app = express();
139+
140+
app.use(function (req, res) {
141+
res.redirect(301, undefined);
142+
});
143+
144+
app.use(function (err, req, res, next) {
145+
res.status(500).json({
146+
error: err.message,
147+
type: err.name
148+
});
149+
});
150+
151+
request(app)
152+
.get('/')
153+
.expect(500)
154+
.expect(function (res) {
155+
if (res.body.type !== 'TypeError') {
156+
throw new Error('Expected TypeError, got ' + res.body.type);
157+
}
158+
if (res.body.error !== 'url argument is required to res.redirect') {
159+
throw new Error('Unexpected error message: ' + res.body.error);
160+
}
161+
})
162+
.end(done);
163+
});
164+
165+
it('should work correctly with valid status and url', function (done) {
166+
var app = express();
167+
168+
app.use(function (req, res) {
169+
res.redirect(301, '/new-location');
170+
});
171+
172+
request(app)
173+
.get('/')
174+
.expect('Location', '/new-location')
175+
.expect(301, done);
176+
});
177+
});
178+
179+
describe('valid redirects should still work', function () {
180+
it('should redirect with a string url', function (done) {
181+
var app = express();
182+
183+
app.use(function (req, res) {
184+
res.redirect('/home');
185+
});
186+
187+
request(app)
188+
.get('/')
189+
.expect('Location', '/home')
190+
.expect(302, done);
191+
});
192+
193+
it('should redirect with empty string', function (done) {
194+
var app = express();
195+
196+
app.use(function (req, res) {
197+
res.redirect('');
198+
});
199+
200+
request(app)
201+
.get('/')
202+
.expect('Location', '')
203+
.expect(302, done);
204+
});
205+
});
206+
});

0 commit comments

Comments
 (0)