Skip to content

[HttpFoundation] Default request _format should be aware of HTTP Accept header #6276

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

Closed

Conversation

stloyd
Copy link
Contributor

@stloyd stloyd commented Dec 12, 2012

Bug fix: no
Feature addition: yes
Backwards compatibility break: yes (?)
Symfony2 tests pass: yes
Fixes the following tickets: #4567

Possible BC break would be in really edge case that is user sets $default format and expects it will be returned but now if Accept header is set, it has priority over default value.

Note: Segfault at Travis-CI is because of some latest changes merged to master, it was passing correctly last days.

@fabpot
Copy link
Member

fabpot commented Dec 12, 2012

This has been discussed a very long time ago and we decided to not support this out of the box as it has many drawbacks (mainly due to the fact that browsers return weird Accept header values -- has it changed now?).

So, if we support this, it should be optional/asked explicitly.

@stloyd
Copy link
Contributor Author

stloyd commented Dec 15, 2012

@fabpot as "optional/asked" you mean something like actual Request::$trustProxy variable or simple extend of method signature to i.e.:

public function getRequestFormat($default = 'html', $useAcceptHeader = false)

@celmaun
Copy link

celmaun commented Dec 15, 2012

@stloyd Have you seen this PR? #5711 It accomplishes HTTP Accept header-aware request _format and more (and is optional).

@fabpot
Copy link
Member

fabpot commented Apr 20, 2013

Closing as it does not play well with browsers.

@fabpot fabpot closed this Apr 20, 2013
@stloyd stloyd deleted the feature/request-format-awareness branch April 21, 2013 10:21
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