Visitar URL original
Don't change relative location header on manual redirect (#1105) · node-fetch/node-fetch@5304f3f · GitHub
Skip to content

Commit 5304f3f

Browse files
tekwiztekwiz
authored
Don't change relative location header on manual redirect (#1105)
* Don't change relative location header on manual redirect * c8 ignores for node-version-specific code and fix c8 ignore in Headers constructor
1 parent f5d3cf5 commit 5304f3f

5 files changed

Lines changed: 49 additions & 9 deletions

File tree

‎README.md‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,23 @@ console.dir(result);
576576

577577
Passed through to the `insecureHTTPParser` option on http(s).request. See [`http.request`](https://nodejs.org/api/http.html#http_http_request_url_options_callback) for more information.
578578

579+
#### Manual Redirect
580+
581+
The `redirect: 'manual'` option for node-fetch is different from the browser & specification, which
582+
results in an [opaque-redirect filtered response](https://fetch.spec.whatwg.org/#concept-filtered-response-opaque-redirect).
583+
node-fetch gives you the typical [basic filtered response](https://fetch.spec.whatwg.org/#concept-filtered-response-basic) instead.
584+
585+
```js
586+
const fetch = require('node-fetch');
587+
588+
const response = await fetch('https://httpbin.org/status/301', { redirect: 'manual' });
589+
590+
if (response.status === 301 || response.status === 302) {
591+
const locationURL = new URL(response.headers.get('location'), response.url);
592+
const response2 = await fetch(locationURL, { redirect: 'manual' });
593+
console.dir(response2);
594+
}
595+
```
579596

580597
<a id="class-request"></a>
581598

‎src/headers.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import {types} from 'node:util';
88
import http from 'node:http';
99

10+
/* c8 ignore next 9 */
1011
const validateHeaderName = typeof http.validateHeaderName === 'function' ?
1112
http.validateHeaderName :
1213
name => {
@@ -17,6 +18,7 @@ const validateHeaderName = typeof http.validateHeaderName === 'function' ?
1718
}
1819
};
1920

21+
/* c8 ignore next 9 */
2022
const validateHeaderValue = typeof http.validateHeaderValue === 'function' ?
2123
http.validateHeaderValue :
2224
(name, value) => {
@@ -141,8 +143,8 @@ export default class Headers extends URLSearchParams {
141143
return Reflect.get(target, p, receiver);
142144
}
143145
}
144-
/* c8 ignore next */
145146
});
147+
/* c8 ignore next */
146148
}
147149

148150
get [Symbol.toStringTag]() {

‎src/index.js‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -154,11 +154,7 @@ export default async function fetch(url, options_) {
154154
finalize();
155155
return;
156156
case 'manual':
157-
// Node-fetch-specific step: make manual redirect a bit easier to use by setting the Location header value to the resolved URL.
158-
if (locationURL !== null) {
159-
headers.set('Location', locationURL);
160-
}
161-
157+
// Nothing to do
162158
break;
163159
case 'follow': {
164160
// HTTP-redirect fetch step 2
@@ -241,6 +237,7 @@ export default async function fetch(url, options_) {
241237

242238
let body = pump(response_, new PassThrough(), reject);
243239
// see https://github.com/nodejs/node/pull/29376
240+
/* c8 ignore next 3 */
244241
if (process.version < 'v12.10') {
245242
response_.on('aborted', abortAndFinalize);
246243
}

‎test/main.js‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,10 @@ describe('node-fetch', () => {
446446
return fetch(url, options).then(res => {
447447
expect(res.url).to.equal(url);
448448
expect(res.status).to.equal(301);
449-
expect(res.headers.get('location')).to.equal(`${base}inspect`);
449+
expect(res.headers.get('location')).to.equal('/inspect');
450+
451+
const locationURL = new URL(res.headers.get('location'), url);
452+
expect(locationURL.href).to.equal(`${base}inspect`);
450453
});
451454
});
452455

@@ -458,7 +461,22 @@ describe('node-fetch', () => {
458461
return fetch(url, options).then(res => {
459462
expect(res.url).to.equal(url);
460463
expect(res.status).to.equal(301);
461-
expect(res.headers.get('location')).to.equal(`${base}redirect/%C3%A2%C2%98%C2%83`);
464+
expect(res.headers.get('location')).to.equal('<>');
465+
466+
const locationURL = new URL(res.headers.get('location'), url);
467+
expect(locationURL.href).to.equal(`${base}redirect/%3C%3E`);
468+
});
469+
});
470+
471+
it('should support redirect mode to other host, manual flag', () => {
472+
const url = `${base}redirect/301/otherhost`;
473+
const options = {
474+
redirect: 'manual'
475+
};
476+
return fetch(url, options).then(res => {
477+
expect(res.url).to.equal(url);
478+
expect(res.status).to.equal(301);
479+
expect(res.headers.get('location')).to.equal('https://github.com/node-fetch');
462480
});
463481
});
464482

‎test/utils/server.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,12 @@ export default class TestServer {
251251
res.end();
252252
}
253253

254+
if (p === '/redirect/301/otherhost') {
255+
res.statusCode = 301;
256+
res.setHeader('Location', 'https://github.com/node-fetch');
257+
res.end();
258+
}
259+
254260
if (p === '/redirect/302') {
255261
res.statusCode = 302;
256262
res.setHeader('Location', '/inspect');
@@ -309,7 +315,7 @@ export default class TestServer {
309315
}
310316

311317
if (p === '/redirect/bad-location') {
312-
res.socket.write('HTTP/1.1 301\r\nLocation: ☃\r\nContent-Length: 0\r\n');
318+
res.socket.write('HTTP/1.1 301\r\nLocation: <>\r\nContent-Length: 0\r\n');
313319
res.socket.end('\r\n');
314320
}
315321

0 commit comments

Comments
 (0)