fix(redirect): Remove authorization header on redirect to different host (#2)

ivanph · zkat · commit 273260e9effe · 2017-05-06T00:45:29.000-07:00
Fixes: #1 Fixes: https://github.com/zkat/pacote/issues/87 Matches the behavior of request: https://github.com/request/request/blob/b12a6245/lib/redirect.js#L134-L138
diff --git a/src/index.js b/src/index.js
@@ -19,6 +19,7 @@ const Headers = require('./headers')
 const Request = require('./request')
 const getNodeRequestOptions = Request.getNodeRequestOptions
 const FetchError = require('./fetch-error')
+const isURL = /^https?:/
 
 /**
  * Fetch function
@@ -86,6 +87,18 @@ function fetch (uri, opts) {
           reject(new FetchError(`redirect location header missing at: ${request.url}`, 'invalid-redirect'))
           return
         }
+        // Remove authorization if changing hostnames (but not if just
+        // changing ports or protocols).  This matches the behavior of request:
+        // https://github.com/request/request/blob/b12a6245/lib/redirect.js#L134-L138
+        let redirectURL = ''
+        if (!isURL.test(res.headers.location)) {
+          redirectURL = url.parse(url.resolve(request.url, res.headers.location))
+        } else {
+          redirectURL = url.parse(res.headers.location)
+        }
+        if (url.parse(request.url).hostname !== redirectURL.hostname) {
+          request.headers.delete('authorization')
+        }
 
         // per fetch spec, for POST request with 301/302 response, or any request with 303 response, use GET when following redirect
         if (res.statusCode === 303 ||
diff --git a/test/server.js b/test/server.js
@@ -252,6 +252,31 @@ module.exports = class TestServer {
 			res.end();
 		}
 
+		if (p === '/redirect/host/different') {
+			res.statusCode = 301;
+			res.setHeader('Location', 'http://127.0.0.1:30001/inspect');
+			res.end();
+		}
+
+		if (p === '/redirect/host/same') {
+			res.statusCode = 301;
+			res.setHeader('Location', 'localhost:30001/inspect');
+			res.end();
+		}
+
+		if (p === '/redirect/host/relativeuri') {
+			res.statusCode = 301;
+			res.setHeader('Location', '/inspect')
+			res.end()
+		}
+
+		if (p === '/redirect/host/protocolrelative') {
+			res.statusCode = 301;
+			res.setHeader('Location', '//localhost:30001/inspect')
+			res.end()
+		}
+
+
 		if (p === '/error/400') {
 			res.statusCode = 400;
 			res.setHeader('Content-Type', 'text/plain');
diff --git a/test/test.js b/test/test.js
@@ -41,6 +41,7 @@ const supportToString = ({
 
 const local = new TestServer();
 const base = `http://${local.hostname}:${local.port}/`;
+const redirectBase = `http://127.0.0.1:${local.port}/`;
 let url, opts;
 
 before(done => {
@@ -251,7 +252,63 @@ describe('node-fetch', () => {
 		});
 	});
 
-	it('should follow POST request redirect code 301 with GET', function() {
+	it('should remove authorization header on redirect if hostname changed', function () {
+		url = `${base}redirect/host/different`
+		opts = {
+			headers: new Headers({ 'authorization': 'abc' })
+		};
+		return fetch(url, opts).then(res => {
+			expect(res.url).to.equal(`${redirectBase}inspect`);
+			return res.json()
+			.then(res => {
+				expect(res.headers['authorization']).to.be.undefined;
+			});
+		});
+	});
+
+	it('should preserve authorization header on redirect if hostname did not change', function () {
+		url = `${base}redirect/host/same`
+		opts = {
+			headers: new Headers({ 'authorization': 'abc' })
+		};
+		return fetch(url, opts).then(res => {
+			expect(res.url).to.equal(`${base}inspect`);
+			return res.json()
+			.then(res => {
+				expect(res.headers['authorization']).to.equal('abc');
+			});
+		});
+	});
+	
+	it('should preserve authorization header on redirect if url is relative', function () {
+		url = `${base}redirect/host/relativeuri`
+		opts = {
+			headers: new Headers({ 'authorization': 'abc' })
+		};
+		return fetch(url, opts).then(res => {
+			expect(res.url).to.equal(`${base}inspect`);
+			return res.json()
+			.then(res => {
+				expect(res.headers['authorization']).to.equal('abc');
+			});
+		});
+	});
+	
+	it('should preserve authorization header on redirect if url is protocol relative', function () {
+		url = `${base}redirect/host/protocolrelative`
+		opts = {
+			headers: new Headers({ 'authorization': 'abc' })
+		};
+		return fetch(url, opts).then(res => {
+			expect(res.url).to.equal(`${base}inspect`);
+			return res.json()
+			.then(res => {
+				expect(res.headers['authorization']).to.equal('abc');
+			});
+		});
+	});
+
+	it('should follow POST request redirect code 301 with GET', function () {
 		url = `${base}redirect/301`;
 		opts = {
 			method: 'POST'
