diff --git a/lib/index.js b/lib/index.js index 1a34953..c5892f4 100755 --- a/lib/index.js +++ b/lib/index.js @@ -19,7 +19,8 @@ const Tap = require('./tap'); const internals = { jsonRegex: /^application\/([a-z0-9.]*[+-]json|json)$/, shallowOptions: ['agent', 'agents', 'beforeRedirect', 'payload', 'redirected'], - httpOptions: ['secureProtocol', 'ciphers', 'lookup', 'family', 'hints'] + httpOptions: ['secureProtocol', 'ciphers', 'lookup', 'family', 'hints'], + sensitiveCrossHostHeaders: new Set(['authorization', 'cookie', 'proxy-authorization']) }; @@ -236,7 +237,7 @@ internals.Client = class { } if (!/^https?:/i.test(location)) { - location = Url.resolve(uri.href, location); + location = new Url.URL(location, uri.href).href; } const redirectOptions = Hoek.clone(options, { shallow: internals.shallowOptions }); @@ -248,13 +249,12 @@ internals.Client = class { redirectOptions.timeout = (redirectOptions.timeout - elapsed).toString(); // stringify to not drop timeout when === 0 } - // When redirecting to a new hostname, remove the authorization and cookie headers + // When redirecting to a new hostname, remove sensitive credential headers if (redirectOptions.headers) { const parsedLocation = new URL(location); if (uri.hostname !== parsedLocation.hostname) { for (const header of Object.keys(redirectOptions.headers)) { - const lowerHeader = header.toLowerCase(); - if (lowerHeader === 'authorization' || lowerHeader === 'cookie') { + if (internals.sensitiveCrossHostHeaders.has(header.toLowerCase())) { delete redirectOptions.headers[header]; } } diff --git a/test/index.js b/test/index.js index 52cc233..9776b6d 100755 --- a/test/index.js +++ b/test/index.js @@ -408,7 +408,7 @@ describe('request()', () => { http2.close(); }); - it('handles redirections with new hostname, removing authorization and cookie headers', async (flags) => { + it('handles redirections with new hostname, removing authorization, cookie, and proxy-authorization headers', async (flags) => { const handler1 = (req, res) => { @@ -418,8 +418,8 @@ describe('request()', () => { const handler2 = (req, res) => { - // request must have 'x-foo' header, but must not have 'authorization' or 'cookie' - if (req.headers.authorization || req.headers.cookie || !req.headers['x-foo']) { + // request must have 'x-foo' header, but must not have 'authorization', 'cookie' or 'proxy-authorization' + if (req.headers.authorization || req.headers.cookie || req.headers['proxy-authorization'] || !req.headers['x-foo']) { res.writeHead(500); } @@ -432,6 +432,7 @@ describe('request()', () => { const headers = { authorization: 'some-auth-key', cookie: 'some-cookie', + 'proxy-authorization': 'some-proxy-auth', 'x-foo': 'something-else' }; @@ -441,6 +442,36 @@ describe('request()', () => { http2.close(); }); + it('preserves proxy-authorization header on same-hostname redirect', async (flags) => { + + let gen = 0; + const handler = (req, res) => { + + if (!gen++) { + res.writeHead(301, { 'Location': '/' }); + res.end(); + return; + } + + // proxy-authorization must persist across same-host redirect + if (req.headers['proxy-authorization'] !== 'some-proxy-auth') { + res.writeHead(500); + } + + res.end(); + }; + + const server = await internals.server(handler); + + const headers = { + 'proxy-authorization': 'some-proxy-auth' + }; + + const res = await Wreck.request('get', 'http://localhost:' + server.address().port, { redirects: 1, headers }); + expect(res.statusCode).to.equal(200); + server.close(); + }); + it('handles redirections from http to https', async (flags) => { const handler = (req, res) => { @@ -1447,9 +1478,13 @@ describe('read()', () => { it('handles requests that close early', async (flags) => { let readPromise; + let readError; const handler = (req, res) => { - readPromise = Wreck.read(req); + readPromise = Wreck.read(req).catch((err) => { + + readError = err; + }); promise.req.abort(); }; @@ -1470,8 +1505,9 @@ describe('read()', () => { const server = await internals.server(handler); const promise = Wreck.request('post', `http://localhost:${server.address().port}`, { payload, headers }); await expect(promise).to.reject(); - const err = await expect(readPromise).to.reject(Error, 'Payload stream closed prematurely'); - expect(err.isBoom).to.equal(true); + await readPromise; + expect(readError).to.be.an.error('Payload stream closed prematurely'); + expect(readError.isBoom).to.equal(true); }); it('errors on partial payload transfers', async (flags) => { @@ -2277,6 +2313,19 @@ describe('toReadableStream()', () => { buf = stream.read(); expect(buf).to.equal(null); }); + + it('does not signal end after a partial _read', () => { + + const data = Buffer.alloc(1000, 'x'); + const stream = Wreck.toReadableStream(data); + + stream._read(400); // partial read leaves position < length + expect(stream._position).to.equal(400); + expect(stream.readableEnded).to.be.false(); + + stream._read(1000); // drains remainder, signals end + expect(stream._position).to.equal(1000); + }); }); describe('Events', () => {