Skip to content
This repository

Infinite loop with 303 #34

Closed
stacia opened this Issue · 6 comments

3 participants

Stacia Mikeal Rogers Loren West
Stacia
stacia commented

I recently had a problem where my app is getting into an infinite loop. I am doing a simple get on a page that returns a 303 redirect. Due to some other bug, the redirect redirects to itself. This leads to an infinite loop and error that doesn't go away and will eventually bring the server down (!).

We are going to fix the bad redirects, but in the meantime we need a way to guard against this. I don't know anything about the way request handles redirects - is there a way to stop it from following the redirect? Initially I thought it must be an error in our code but going through the loop in the debugger was jumping between the error and a line in request that was doing callback if options.callback was set with error. That callback keeps getting set and I can't stop it - I just want to return when I encounter an issue like this.

Here's the error:

Error: Exceeded maxRedirects. Probably stuck in a redirect loop.
at ClientRequest. ((..)node_modules/request/main.js:215:31)
at ClientRequest.g (events.js:143:14)
at ClientRequest.emit (events.js:64:17)
at HTTPParser.onIncoming (http.js:1349:9)
at HTTPParser.onHeadersComplete (http.js:108:31)
at Socket.ondata (http.js:1226:22)
at Socket._onReadable (net.js:683:27)
at IOWatcher.onReadable as callback

Mikeal Rogers
Owner
mikeal commented
Stacia
stacia commented

I was able to reproduce the error with this simple example:

var Request = require('request');
var x = (create a dummy server that has a 303 that directs to another url that has a 303 directing back to the first one)
Request({uri: x, method : "GET"}, function(error, body){
console.log("returned")
})

The console just displays "returned" forever.

Loren West

This indeed is an error in the request library in that it detects the max redirects, but happily continues processing redirects until the VM runs out of memory, and your logfiles are filled up with hundreds of thousands of "Exceeded redirects" messages.

Fortunately the fix is easy:

In main.js at line 224, right after the options.emit('error'... just add a return statement.

Mikeal Rogers
Owner
mikeal commented

we should add a test for this

Loren West

Indeed. It would require someone to set up a server that causes this bad behavior though.

Mikeal Rogers
Owner
mikeal commented

we fixed this right? i can't remember. we do need a test eventually but i want to release 2.0 soon.

Mikeal Rogers mikeal closed this in 411b30d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.