Skip to content

Commit 36988e8

Browse files
authored
Prevent open redirects when cleanUrls config is enabled (#122)
* Prevent open redirects when `cleanUrls` config is enabled * Cov
1 parent b0f1a62 commit 36988e8

File tree

2 files changed

+22
-10
lines changed

2 files changed

+22
-10
lines changed

src/index.js

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -127,15 +127,19 @@ const shouldRedirect = (decodedPath, {redirects = [], trailingSlash}, cleanUrl)
127127
return null;
128128
}
129129

130-
let cleanedUrl = false;
131-
132130
// By stripping the HTML parts from the decoded
133131
// path *before* handling the trailing slash, we make
134132
// sure that only *one* redirect occurs if both
135133
// config options are used.
136134
if (cleanUrl && matchHTML.test(decodedPath)) {
137135
decodedPath = decodedPath.replace(matchHTML, '');
138-
cleanedUrl = true;
136+
if (decodedPath.indexOf('//') > -1) {
137+
decodedPath = decodedPath.replace(/\/+/g, '/');
138+
}
139+
return {
140+
target: ensureSlashStart(decodedPath),
141+
statusCode: defaultType
142+
};
139143
}
140144

141145
if (slashing) {
@@ -163,13 +167,6 @@ const shouldRedirect = (decodedPath, {redirects = [], trailingSlash}, cleanUrl)
163167
}
164168
}
165169

166-
if (cleanedUrl) {
167-
return {
168-
target: ensureSlashStart(decodedPath),
169-
statusCode: defaultType
170-
};
171-
}
172-
173170
// This is currently the fastest way to
174171
// iterate over an array
175172
for (let index = 0; index < redirects.length; index++) {
@@ -412,6 +409,7 @@ const renderDirectory = async (current, acceptsJSON, handlers, methods, config,
412409
return 1;
413410
}
414411

412+
/* istanbul ignore next */
415413
if (a.base < b.base) {
416414
return -1;
417415
}

test/integration.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,20 @@ test('set `trailingSlash` config property to `false`', async t => {
278278
t.is(location, target);
279279
});
280280

281+
test('set `cleanUrls` config property should prevent open redirects', async t => {
282+
const url = await getUrl({
283+
cleanUrls: true
284+
});
285+
286+
const response = await fetch(`${url}//haveibeenpwned.com/index`, {
287+
redirect: 'manual',
288+
follow: 0
289+
});
290+
291+
const location = response.headers.get('location');
292+
t.is(location, `${url}/haveibeenpwned.com`);
293+
});
294+
281295
test('set `rewrites` config property to wildcard path', async t => {
282296
const destination = '.dotfile';
283297
const related = path.join(fixturesFull, destination);

0 commit comments

Comments
 (0)