Skip to content

Commit 472ed9a

Browse files
committed
fix: fixed max redirects and xhr/fetch issue
1 parent 49206ed commit 472ed9a

File tree

2 files changed

+24
-27
lines changed

2 files changed

+24
-27
lines changed

src/index.js

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,17 @@ module.exports = opts => {
77
maxRedirects: 5,
88
...opts
99
};
10-
const { defaultPath, maxRedirects } = opts;
1110
return (req, res, next) => {
1211
if (!req.session)
1312
return next(new Error('Sessions required for `express-redirect-loop`'));
1413

1514
const { redirect, end } = res;
1615

1716
res.end = function(chunk, encoding) {
18-
if (!req.xhr) {
17+
// instead of `!req.xhr` we need to use !accepts HTML
18+
// because Fetch does not provide XMLHttpRequest
19+
if (req.accepts('html')) {
1920
req.session.prevPrevPath = req.session.prevPath;
20-
req.session.prevPrevMethod = req.session.prevMethod;
2121
req.session.prevPath = req.originalUrl;
2222
req.session.prevMethod = req.method;
2323
// if it was a redirect then store how many times
@@ -53,30 +53,27 @@ module.exports = opts => {
5353

5454
address = this.location(address).get('Location');
5555

56-
req.prevPrevPath = req.session.prevPrevPath || defaultPath;
57-
req.prevPrevMethod = req.session.prevPrevMethod || 'GET';
58-
req.prevPath = req.session.prevPath || defaultPath;
59-
req.prevMethod = req.session.prevMethod || req.method;
60-
req.maxRedirects = req.session.maxRedirects || 1;
56+
const prevPrevPath = req.session.prevPrevPath || opts.defaultPath;
57+
const prevPath = req.session.prevPath || opts.defaultPath;
58+
const prevMethod = req.session.prevMethod || req.method;
59+
const maxRedirects = req.session.maxRedirects || 1;
6160

62-
if (
63-
req.prevPath &&
64-
address === req.prevPath &&
65-
req.method === req.prevMethod
66-
) {
61+
if (prevPath && address === prevPath && req.method === prevMethod) {
6762
if (
68-
req.prevPrevPath &&
69-
address !== req.prevPrevPath &&
70-
req.maxRedirects <= maxRedirects
63+
prevPrevPath &&
64+
address !== prevPrevPath &&
65+
maxRedirects <= opts.maxRedirects
7166
) {
72-
address = req.prevPrevPath;
67+
address = prevPrevPath;
7368
} else {
7469
// if the prevPrevPath w/o querystring is !== prevPrevPath
7570
// then redirect then to prevPrevPath w/o querystring
76-
const { pathname } = new Url(req.prevPrevPath, {});
77-
if (pathname === req.prevPrevPath) address = '/';
71+
const { pathname } = new Url(prevPrevPath, {});
72+
if (pathname === prevPrevPath) address = '/';
7873
else address = pathname;
7974
}
75+
} else if (maxRedirects > opts.maxRedirects) {
76+
address = opts.defaultPath;
8077
}
8178

8279
redirect.call(res, status, address);

test/test.js

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,13 @@ test.beforeEach(t => {
2525
app.get('/baz', (req, res) => res.redirect('/bar'));
2626
app.get('/beep', (req, res) => res.sendStatus(200));
2727
app.get('/boop', (req, res) => res.redirect('/boop'));
28-
app.get('/1', (req, res) => res.redirect('/2'));
29-
app.get('/2', (req, res) => res.redirect('/3'));
30-
app.get('/3', (req, res) => res.redirect('/4'));
31-
app.get('/4', (req, res) => res.redirect('/4')); // <-- should be 5
32-
app.get('/5', (req, res) => res.redirect('/6'));
33-
app.get('/6', (req, res) => res.redirect('/7'));
28+
app.get('/1', (req, res) => res.redirect('/2')); // 1
29+
app.get('/2', (req, res) => res.redirect('/3')); // 2
30+
app.get('/3', (req, res) => res.redirect('/4')); // 3
31+
app.get('/4', (req, res) => res.redirect('/5')); // 4
32+
app.get('/5', (req, res) => res.redirect('/6')); // 5
33+
app.get('/6', (req, res) => res.redirect('/7')); // 6 <-- redirects to /
34+
app.get('/7', (req, res) => res.redirect('/8'));
3435
app.get('/form', (req, res) => res.sendStatus(200));
3536
app.post('/form', (req, res) => res.redirect('/form'));
3637
app.use((err, req, res, next) => {
@@ -45,9 +46,8 @@ test('caps at max of 5 redirects', async t => {
4546
const res = await fetch(`${t.context.url}1`, {
4647
credentials: 'include'
4748
});
48-
console.log('res', res, 'res.body', res.body);
4949
t.is(res.status, 200);
50-
t.is(res.url, t.context.url);
50+
t.is(res.url, `${t.context.url}`);
5151
t.pass();
5252
});
5353

0 commit comments

Comments
 (0)