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

Fix : Redirect to request Uri instead of pathInfo #97

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vrcAlbert
Copy link

Hello,

Asking for http://hostname.dev/app_dev.php/path redirects to http://m.hostname.dev/path and does not preserve app_dev.php.

Is there any reason for using pathInfo instead of request URI ?

If not, here's the change you could merge.

Thanks in advance for your time and answer.

@ogizanagi
Copy link

ogizanagi commented Aug 8, 2016

@vrcAlbert : The issue however is that the Request::getRequestUri() method also returns the query string.

You may want to remove this line:

- Request::normalizeQueryString(http_build_query($queryParams, null, '&'))

But still, l.287 is added the switch param, which is not part of the current request.

So, I'd suggest to simply concatenate the result of the Request::getBaseUrl() method instead.

This fix should also be applied to the RequestResponseListener::REDIRECT_WITHOUT_PATH case.

fix : remove query string from request URI in redirect response
@vrcAlbert
Copy link
Author

Thanks @ogizanagi for pointing that out !

Concerning REDIRECT_WITHOUT_PATH, there could be a case in which we would use that option to explicitely remove the base script. I'll let @suncat2000 giving his final word about it :)

Thanks again for the review.

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.

2 participants