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

Do not use dynamic REQUEST_URI from $_SERVER as base url #5652

Merged
merged 1 commit into from
Sep 8, 2015

Conversation

senkal
Copy link
Contributor

@senkal senkal commented Aug 23, 2015

Q A
Doc fix? yes
New docs? no
Applies to 2.3
Fixed tickets

I am not sure why we use $_SERVER['REQUEST_URI'] as a first parameter for Symfony\Component\Routing\RequestContext where it expects to get baseUrl.
$_SERVER['REQUEST_URI'] is dynamic and shouldn't be used as a static value for baseUrl.

Probably better not to pass this parameter at all(default) or use /, what I propose.
I know that many people won't look that deeply into those examples but, in my opinion, $_SERVER['REQUEST_URI'] is not base url.

Not sure what to do with the note about the request uri(the one with http foundation link/info).

Maybe the idea was to use $_SERVER['REQUEST_URI'] in ->match() examples ?

@senkal senkal changed the title Od not use dynamic REQUEST_URI from $_SERVER as base url Do not use dynamic REQUEST_URI from $_SERVER as base url Aug 23, 2015
@xabbuh
Copy link
Member

xabbuh commented Sep 5, 2015

@weaverryan It seems like you did that initial change. Do you know why $_SERVER['REQUEST_URI'] was used instead of /?

@weaverryan
Copy link
Member

:thumbs: I agree - I think I just made a quick mistake - I can't remember anything specific related to this

@xabbuh
Copy link
Member

xabbuh commented Sep 8, 2015

👍

@xabbuh
Copy link
Member

xabbuh commented Sep 8, 2015

Thank you @senkal.

@xabbuh xabbuh merged commit dadefb6 into symfony:2.3 Sep 8, 2015
xabbuh added a commit that referenced this pull request Sep 8, 2015
…(senkal)

This PR was merged into the 2.3 branch.

Discussion
----------

Do not use dynamic REQUEST_URI from $_SERVER as base url

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | 2.3
| Fixed tickets |

I am not sure why we use `$_SERVER['REQUEST_URI']` as a first parameter for `Symfony\Component\Routing\RequestContext` where it expects to get baseUrl.
`$_SERVER['REQUEST_URI'] ` is dynamic and shouldn't be used as a static value for baseUrl.

Probably better not to pass this parameter at all(default) or use `/`, what I propose.
I know that many people won't look that deeply into those examples but, in my opinion, `$_SERVER['REQUEST_URI'] ` is not base url.

Not sure what to do with the note about the request uri(the one with http foundation link/info).

Maybe the idea was to use `$_SERVER['REQUEST_URI']` in `->match()` examples ?

Commits
-------

dadefb6 Od not use dynamic REQUEST_URI from $_SERVER as base url
@senkal senkal deleted the routing-component-base-url-example-fix branch September 8, 2015 18:43
xabbuh added a commit that referenced this pull request Oct 4, 2015
…g component (senkal)

This PR was squashed before being merged into the 2.3 branch (closes #5678).

Discussion
----------

Update HttpFoundation note after recent changes in routing component

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | 2.3
| Fixed tickets |

Update HttpFoundation note to reflect changes in #5652.
We do not longer use `$_SERVER['REQUEST_URI']` in examples so keeping the reference in note doesn't have much value.
I updated it to make it more generic and reflect current examples.

Commits
-------

36234f2 Update HttpFoundation note after recent changes in routing component
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants