fix(Headers): don't forward secure headers on protocol change (#1599)

* fix(Headers): don't forward secure headers on protocol change

* fix lint

Author: max-stytch

src/index.js

@@ -22,7 +22,7 @@ import {FetchError} from './errors/fetch-error.js';
 import {AbortError} from './errors/abort-error.js';
 import {isRedirect} from './utils/is-redirect.js';
 import {FormData} from 'formdata-polyfill/esm.min.js';
-import {isDomainOrSubdomain} from './utils/is.js';
+import {isDomainOrSubdomain, isSameProtocol} from './utils/is.js';
 import {parseReferrerPolicyFromHeader} from './utils/referrer.js';
 import {
 	Blob,
@@ -203,7 +203,10 @@ export default async function fetch(url, options_) {
 						// that is not a subdomain match or exact match of the initial domain.
 						// For example, a redirect from "foo.com" to either "foo.com" or "sub.foo.com"
 						// will forward the sensitive headers, but a redirect to "bar.com" will not.
-						if (!isDomainOrSubdomain(request.url, locationURL)) {
+						// headers will also be ignored when following a redirect to a domain using
+						// a different protocol. For example, a redirect from "https://foo.com" to "http://foo.com"
+						// will not forward the sensitive headers
+						if (!isDomainOrSubdomain(request.url, locationURL) || !isSameProtocol(request.url, locationURL)) {
 							for (const name of ['authorization', 'www-authenticate', 'cookie', 'cookie2']) {
 								requestOptions.headers.delete(name);
 							}

src/utils/is.js

@@ -71,3 +71,17 @@ export const isDomainOrSubdomain = (destination, original) => {
 
 	return orig === dest || orig.endsWith(`.${dest}`);
 };
+
+/**
+ * isSameProtocol reports whether the two provided URLs use the same protocol.
+ *
+ * Both domains must already be in canonical form.
+ * @param {string|URL} original
+ * @param {string|URL} destination
+ */
+export const isSameProtocol = (destination, original) => {
+	const orig = new URL(original).protocol;
+	const dest = new URL(destination).protocol;
+
+	return orig === dest;
+};

test/main.js

@@ -33,7 +33,7 @@ import ResponseOrig from '../src/response.js';
 import Body, {getTotalBytes, extractContentType} from '../src/body.js';
 import TestServer from './utils/server.js';
 import chaiTimeout from './utils/chai-timeout.js';
-import {isDomainOrSubdomain} from '../src/utils/is.js';
+import {isDomainOrSubdomain, isSameProtocol} from '../src/utils/is.js';
 
 const AbortControllerPolyfill = abortControllerPolyfill.AbortController;
 const encoder = new TextEncoder();
@@ -522,7 +522,7 @@ describe('node-fetch', () => {
 		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
+		// Unsafe headers are not removed
 		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');
@@ -542,6 +542,39 @@ describe('node-fetch', () => {
 		expect(isDomainOrSubdomain('http://bob.uk.com', 'http://xyz.uk.com')).to.be.false;
 	});
 
+	it('should not forward secure headers to changed protocol', async () => {
+		const res = await fetch('https://httpbin.org/redirect-to?url=http%3A%2F%2Fhttpbin.org%2Fget&status_code=302', {
+			headers: new Headers({
+				cookie: 'gets=removed',
+				cookie2: 'gets=removed',
+				authorization: 'gets=removed',
+				'www-authenticate': 'gets=removed',
+				'other-safe-headers': 'stays',
+				'x-foo': 'bar'
+			})
+		});
+
+		const headers = new Headers((await res.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 downgraded http
+		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('isSameProtocol', () => {
+		// Forwarding headers to same protocol is OK
+		expect(isSameProtocol('http://a.com', 'http://a.com')).to.be.true;
+		expect(isSameProtocol('https://a.com', 'https://www.a.com')).to.be.true;
+
+		// Forwarding headers to diff protocol is not OK
+		expect(isSameProtocol('http://b.com', 'https://b.com')).to.be.false;
+		expect(isSameProtocol('http://www.a.com', 'https://a.com')).to.be.false;
+	});
+
 	it('should treat broken redirect as ordinary response (follow)', async () => {
 		const url = `${base}redirect/no-location`;
 		const res = await fetch(url);