From 7d0ea4bfc87538a722f19bd1affccac8d20c2d7f Mon Sep 17 00:00:00 2001 From: Louis Buchbinder Date: Sat, 27 Apr 2019 20:51:50 -0700 Subject: [PATCH] Invalid redirect in pipe (#1471) * do not throw on bad redirect in pipe * linting --- .gitignore | 1 + src/node/index.js | 10 ++++++--- test/node/pipe.js | 53 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index ef4ee3814..1592c7754 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,4 @@ coverage .nyc_output lib dist +*.swp diff --git a/src/node/index.js b/src/node/index.js index 3722da46b..23e478272 100644 --- a/src/node/index.js +++ b/src/node/index.js @@ -423,9 +423,13 @@ Request.prototype.pipe = function(stream, options) { Request.prototype._pipeContinue = function(stream, options) { this.req.once('response', res => { // redirect - const redirect = isRedirect(res.statusCode); - if (redirect && this._redirects++ !== this._maxRedirects) { - return this._redirect(res)._pipeContinue(stream, options); + if ( + isRedirect(res.statusCode) && + this._redirects++ !== this._maxRedirects + ) { + return this._redirect(res) === this + ? this._pipeContinue(stream, options) + : undefined; } this.res = res; diff --git a/test/node/pipe.js b/test/node/pipe.js index 23ca369ba..08bcbdc2c 100644 --- a/test/node/pipe.js +++ b/test/node/pipe.js @@ -17,6 +17,14 @@ app.get('/', (req, res) => { fs.createReadStream('test/node/fixtures/user.json').pipe(res); }); +app.get('/redirect', (req, res) => { + res.set('Location', '/').sendStatus(302); +}); + +app.get('/badRedirectNoLocation', (req, res) => { + res.set('Location', '').sendStatus(302); +}); + app.post('/', (req, res) => { if (process.env.HTTP2_TEST) { // body-parser does not support http2 yet. @@ -93,4 +101,49 @@ describe('request pipe', () => { }); req.pipe(stream); }); + + it('should follow redirects', done => { + const stream = fs.createWriteStream(destPath); + + let responseCalled = false; + const req = request.get(base + '/redirect'); + req.type('json'); + + req.on('response', res => { + res.status.should.eql(200); + responseCalled = true; + }); + stream.on('finish', () => { + JSON.parse(fs.readFileSync(destPath, 'utf8')).should.eql({ + name: 'tobi' + }); + responseCalled.should.be.true(); + done(); + }); + req.pipe(stream); + }); + + it('should not throw on bad redirects', done => { + const stream = fs.createWriteStream(destPath); + + let responseCalled = false; + let errorCalled = false; + const req = request.get(base + '/badRedirectNoLocation'); + req.type('json'); + + req.on('response', res => { + responseCalled = true; + }); + req.on('error', err => { + err.message.should.eql('No location header for redirect'); + errorCalled = true; + stream.end(); + }); + stream.on('finish', () => { + responseCalled.should.be.false(); + errorCalled.should.be.true(); + done(); + }); + req.pipe(stream); + }); });