backport of #1449 (#1453)

jimmywarting · web-flow · commit 1ef4b560a17e · 2022-01-16T12:45:33.000+01:00
* backport of #1449 * bump patch version
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
     "name": "node-fetch",
-    "version": "2.6.6",
+    "version": "2.6.7",
     "description": "A light-weight module that brings window.fetch to node.js",
     "main": "lib/index.js",
     "browser": "./browser.js",
diff --git a/src/index.js b/src/index.js
@@ -13,16 +13,29 @@ import https from 'https';
 import zlib from 'zlib';
 import Stream from 'stream';
 
-import Body, { writeToStream, getTotalBytes } from './body';
-import Response from './response';
-import Headers, { createHeadersLenient } from './headers';
-import Request, { getNodeRequestOptions } from './request';
-import FetchError from './fetch-error';
-import AbortError from './abort-error';
+import Body, { writeToStream, getTotalBytes } from './body.js';
+import Response from './response.js';
+import Headers, { createHeadersLenient } from './headers.js';
+import Request, { getNodeRequestOptions } from './request.js';
+import FetchError from './fetch-error.js';
+import AbortError from './abort-error.js';
+
+import whatwgUrl from 'whatwg-url';
+
+const URL = Url.URL || whatwgUrl.URL;
 
 // fix an issue where "PassThrough", "resolve" aren't a named export for node <10
 const PassThrough = Stream.PassThrough;
-const resolve_url = Url.resolve;
+
+const isDomainOrSubdomain = (destination, original) => {
+	const orig = new URL(original).hostname;
+	const dest = new URL(destination).hostname;
+
+	return orig === dest || (
+		orig[orig.length - dest.length - 1] === '.' && orig.endsWith(dest)
+	);
+};
+
 
 /**
  * Fetch function
@@ -109,7 +122,19 @@ export default function fetch(url, opts) {
 				const location = headers.get('Location');
 
 				// HTTP fetch step 5.3
-				const locationURL = location === null ? null : resolve_url(request.url, location);
+				let locationURL = null;
+				try {
+					locationURL = location === null ? null : new URL(location, request.url).toString();
+				} catch (err) {
+					// error here can only be invalid URL in Location: header
+					// do not throw when options.redirect == manual
+					// let the user extract the errorneous redirect URL
+					if (request.redirect !== 'manual') {
+						reject(new FetchError(`uri requested responds with an invalid redirect URL: ${location}`, 'invalid-redirect'));
+						finalize();
+						return;
+					}
+				}
 
 				// HTTP fetch step 5.5
 				switch (request.redirect) {
@@ -154,9 +179,15 @@ export default function fetch(url, opts) {
 							body: request.body,
 							signal: request.signal,
 							timeout: request.timeout,
-                            size: request.size
+              size: request.size
 						};
 
+						if (!isDomainOrSubdomain(request.url, locationURL)) {
+							for (const name of ['authorization', 'www-authenticate', 'cookie', 'cookie2']) {
+								requestOpts.headers.delete(name);
+							}
+						}
+
 						// HTTP-redirect fetch step 9
 						if (res.statusCode !== 303 && request.body && getTotalBytes(request) === null) {
 							reject(new FetchError('Cannot follow redirect with body being a readable stream', 'unsupported-redirect'));
diff --git a/test/server.js b/test/server.js
@@ -1,7 +1,6 @@
 import * as http from 'http';
 import { parse } from 'url';
 import * as zlib from 'zlib';
-import * as stream from 'stream';
 import { multipart as Multipart } from 'parted';
 
 let convert;
@@ -66,6 +65,12 @@ export default class TestServer {
 			}));
 		}
 
+		if (p.startsWith('/redirect-to/3')) {
+			res.statusCode = p.slice(13, 16);
+			res.setHeader('Location', p.slice(17));
+			res.end();
+		}
+
 		if (p === '/gzip') {
 			res.statusCode = 200;
 			res.setHeader('Content-Type', 'text/plain');
diff --git a/test/test.js b/test/test.js
@@ -1569,6 +1569,53 @@ describe('node-fetch', () => {
 		});
 	});
 
+	it('should not forward secure headers to 3th party', () => {
+		return fetch(`${base}redirect-to/302/https://httpbin.org/get`, {
+			headers: new Headers({
+				cookie: 'gets=removed',
+				cookie2: 'gets=removed',
+				authorization: 'gets=removed',
+				'www-authenticate': 'gets=removed',
+				'other-safe-headers': 'stays',
+				'x-foo': 'bar'
+			})
+		}).then(res => res.json()).then(json => {
+			const headers = new Headers(json.headers);
+			// Safe headers are not removed
+			expect(headers.get('other-safe-headers')).to.equal('stays');
+			expect(headers.get('x-foo')).to.equal('bar');
+			// Unsafe headers should not have been sent to httpbin
+			expect(headers.get('cookie')).to.equal(null);
+			expect(headers.get('cookie2')).to.equal(null);
+			expect(headers.get('www-authenticate')).to.equal(null);
+			expect(headers.get('authorization')).to.equal(null);
+		});
+	});
+
+	it('should forward secure headers to same host', () => {
+		return fetch(`${base}redirect-to/302/${base}inspect`, {
+			headers: new Headers({
+				cookie: 'is=cookie',
+				cookie2: 'is=cookie2',
+				authorization: 'is=authorization',
+				'other-safe-headers': 'stays',
+				'www-authenticate': 'is=www-authenticate',
+				'x-foo': 'bar'
+			})
+		}).then(res => res.json().then(json => {
+			const headers = new Headers(json.headers);
+			// Safe headers are not removed
+			expect(res.url).to.equal(`${base}inspect`);
+			expect(headers.get('other-safe-headers')).to.equal('stays');
+			expect(headers.get('x-foo')).to.equal('bar');
+			// Unsafe headers should not have been sent to httpbin
+			expect(headers.get('cookie')).to.equal('is=cookie');
+			expect(headers.get('cookie2')).to.equal('is=cookie2');
+			expect(headers.get('www-authenticate')).to.equal('is=www-authenticate');
+			expect(headers.get('authorization')).to.equal('is=authorization');
+		}));
+	});
+
 	it('should allow PATCH request', function() {
 		const url = `${base}inspect`;
 		const opts = {

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "node-fetch",`
`3`		`- "version": "2.6.6",`
	`3`	`+ "version": "2.6.7",`
`4`	`4`	`"description": "A light-weight module that brings window.fetch to node.js",`
`5`	`5`	`"main": "lib/index.js",`
`6`	`6`	`"browser": "./browser.js",`