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

"Custom" URL rewrites error out. #2

Closed
danemacmillan opened this issue Nov 28, 2014 · 2 comments
Closed

"Custom" URL rewrites error out. #2

danemacmillan opened this issue Nov 28, 2014 · 2 comments

Comments

@danemacmillan
Copy link

This module behaves like another when it comes to "Custom" URL rewrite types in the "URL Rewrite Management" section of the admin: jreinke/magento-hide-default-store-code#2 (comment).

The exception being that this one throws an "Invalid HTTP response code" 503 error, while the other throws a 404.

@danemacmillan
Copy link
Author

I've narrowed it down.

The router is sending an invalid HTTP status code here:
https://github.com/Knectar/Magento-Store-Codes/blob/master/app/code/community/Knectar/Storecodes/Controller/Router.php#L89

The $redirect variable is receiving an HTTP status code of 0. Now, let's look at how that code is generated.

The ternary logic here https://github.com/Knectar/Magento-Store-Codes/blob/master/app/code/community/Knectar/Storecodes/Helper/Data.php#L40 says that if $code is 1 (based on the boolean config that determines whether URLs should be directed or not) use a proper HTTP status code, but if it's not, use the boolean config value, being 0. 0 is obviously not a valid HTTP status code. Shouldn't it be 200, given that there are only two meanings to the option that can be set in the admin? One possibility would be to keep the store code in the URL, thus do nothing but return (200, or even 301) the page, or redirect it elsewhere, thus send a 302.

What do you think?

@clockworkgeek
Copy link
Contributor

I could not recreate the error on my test rig, but there could be other reasons for that. Your explanation makes sense so I fixed it. Thank you for finding a bug so soon after it emerged.

When the config is 0 the behaviour is not to to redirect with a HTTP code of 200, because 200 does not mean redirect. Instead I avoid the whole if clause if the "auto-redirect" option is "No".

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

No branches or pull requests

2 participants