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

README.md

@@ -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>
 

src/headers.js

@@ -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 => {
@@ -17,6 +18,7 @@ const validateHeaderName = typeof http.validateHeaderName === 'function' ?
 		}
 	};
 
+/* c8 ignore next 9 */
 const validateHeaderValue = typeof http.validateHeaderValue === 'function' ?
 	http.validateHeaderValue :
 	(name, value) => {
@@ -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]() {

src/index.js

@@ -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
@@ -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);
 			}

test/main.js

@@ -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`);
 		});
 	});
 
@@ -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');
 		});
 	});
 

test/utils/server.js

@@ -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');
@@ -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');
 		}