Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't change relative location header on manual redirect #1105

Merged
merged 2 commits into from Jan 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 17 additions & 0 deletions README.md
Expand Up @@ -576,6 +576,23 @@ console.dir(result);

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.

#### Manual Redirect

The `redirect: 'manual'` option for node-fetch is different from the browser & specification, which
results in an [opaque-redirect filtered response](https://fetch.spec.whatwg.org/#concept-filtered-response-opaque-redirect).
node-fetch gives you the typical [basic filtered response](https://fetch.spec.whatwg.org/#concept-filtered-response-basic) instead.

```js
const fetch = require('node-fetch');

const response = await fetch('https://httpbin.org/status/301', { redirect: 'manual' });

if (response.status === 301 || response.status === 302) {
const locationURL = new URL(response.headers.get('location'), response.url);
const response2 = await fetch(locationURL, { redirect: 'manual' });
console.dir(response2);
}
```

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

Expand Down
4 changes: 3 additions & 1 deletion src/headers.js
Expand Up @@ -7,6 +7,7 @@
import {types} from 'node:util';
import http from 'node:http';

/* c8 ignore next 9 */
const validateHeaderName = typeof http.validateHeaderName === 'function' ?
http.validateHeaderName :
name => {
Expand All @@ -17,6 +18,7 @@ const validateHeaderName = typeof http.validateHeaderName === 'function' ?
}
};

/* c8 ignore next 9 */
const validateHeaderValue = typeof http.validateHeaderValue === 'function' ?
http.validateHeaderValue :
(name, value) => {
Expand Down Expand Up @@ -141,8 +143,8 @@ export default class Headers extends URLSearchParams {
return Reflect.get(target, p, receiver);
}
}
/* c8 ignore next */
});
/* c8 ignore next */
}

get [Symbol.toStringTag]() {
Expand Down
7 changes: 2 additions & 5 deletions src/index.js
Expand Up @@ -154,11 +154,7 @@ export default async function fetch(url, options_) {
finalize();
return;
case 'manual':
// Node-fetch-specific step: make manual redirect a bit easier to use by setting the Location header value to the resolved URL.
if (locationURL !== null) {
headers.set('Location', locationURL);
}

// Nothing to do
break;
case 'follow': {
// HTTP-redirect fetch step 2
Expand Down Expand Up @@ -241,6 +237,7 @@ export default async function fetch(url, options_) {

let body = pump(response_, new PassThrough(), reject);
// see https://github.com/nodejs/node/pull/29376
/* c8 ignore next 3 */
if (process.version < 'v12.10') {
response_.on('aborted', abortAndFinalize);
}
Expand Down
22 changes: 20 additions & 2 deletions test/main.js
Expand Up @@ -446,7 +446,10 @@ describe('node-fetch', () => {
return fetch(url, options).then(res => {
expect(res.url).to.equal(url);
expect(res.status).to.equal(301);
expect(res.headers.get('location')).to.equal(`${base}inspect`);
expect(res.headers.get('location')).to.equal('/inspect');

const locationURL = new URL(res.headers.get('location'), url);
expect(locationURL.href).to.equal(`${base}inspect`);
});
});

Expand All @@ -458,7 +461,22 @@ describe('node-fetch', () => {
return fetch(url, options).then(res => {
expect(res.url).to.equal(url);
expect(res.status).to.equal(301);
expect(res.headers.get('location')).to.equal(`${base}redirect/%C3%A2%C2%98%C2%83`);
expect(res.headers.get('location')).to.equal('<>');

const locationURL = new URL(res.headers.get('location'), url);
expect(locationURL.href).to.equal(`${base}redirect/%3C%3E`);
});
});

it('should support redirect mode to other host, manual flag', () => {
const url = `${base}redirect/301/otherhost`;
const options = {
redirect: 'manual'
};
return fetch(url, options).then(res => {
expect(res.url).to.equal(url);
expect(res.status).to.equal(301);
expect(res.headers.get('location')).to.equal('https://github.com/node-fetch');
});
});

Expand Down
8 changes: 7 additions & 1 deletion test/utils/server.js
Expand Up @@ -251,6 +251,12 @@ export default class TestServer {
res.end();
}

if (p === '/redirect/301/otherhost') {
res.statusCode = 301;
res.setHeader('Location', 'https://github.com/node-fetch');
res.end();
}

if (p === '/redirect/302') {
res.statusCode = 302;
res.setHeader('Location', '/inspect');
Expand Down Expand Up @@ -309,7 +315,7 @@ export default class TestServer {
}

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

Expand Down