Skip to content

Commit

Permalink
Invalid redirect in pipe (#1471)
Browse files Browse the repository at this point in the history
* do not throw on bad redirect in pipe

* linting
  • Loading branch information
louisbuchbinder authored and niftylettuce committed Apr 28, 2019
1 parent 251a1cb commit 7d0ea4b
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 3 deletions.
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -15,3 +15,4 @@ coverage
.nyc_output
lib
dist
*.swp
10 changes: 7 additions & 3 deletions src/node/index.js
Expand Up @@ -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;
Expand Down
53 changes: 53 additions & 0 deletions test/node/pipe.js
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
});
});

0 comments on commit 7d0ea4b

Please sign in to comment.