Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH prevent redirect loop #105

Closed
wants to merge 1 commit into from
Closed

Conversation

IgorNadj
Copy link

To reproduce, rename your framework directory to framework2 (to simulate a missing framework) and make a http request.

Before change, the reponse is 500 Internal Server Error, and the error is (apache/error.log):
"AH00124: Request exceeded the limit of 10 internal redirects due to probable configuration error. ..."

After change, the response is 404 Not Found, and contains helpful error message:
"The requested URL /framework/main.php was not found on this server"

Note: I'm not sure exactly where this code should go, if it's the first rule or after http basic auth etc.

@tractorcow
Copy link

The correct response code is 500; The server is incorrectly configured.

A 404 implies that the user requested a url where no resource exists.

@IgorNadj
Copy link
Author

It's true that the 404 is misleading, however an apache error is misleading as well. It's an app error really, the app's .htaccess is not doing anything useful, but instead getting stuck in a redirect loop.

I've just seen the file install-frameworkmissing.html, perhaps it should redirect there?

## Internal Redirect Loop Protection
    RewriteCond %{ENV:REDIRECT_STATUS} 200
    RewriteRule ^ install-frameworkmissing.html [L]

@dhensby
Copy link
Contributor

dhensby commented Oct 14, 2015

That latest suggestion makes sense.

@IgorNadj
Copy link
Author

This seems to false-alarm. Very dangerous to put in, for relatively minor benefits. Closing PR.

@IgorNadj IgorNadj closed this May 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants