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

[FrameworkBundle] Add support to 307/308 HTTP status codes in RedirectController #26213

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@ZipoKing
Contributor

ZipoKing commented Feb 18, 2018

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #26171
License MIT
Doc PR

With this PR RedirectController will allow to create redirections with use of 307/308 HTTP status codes together with 301/302. Related RFC documents:

@nicolas-grekas

thanks for working on this; see comments

*
* @throws HttpException In case the route name is empty
*/
public function redirectAction(Request $request, string $route, bool $permanent = false, $ignoreAttributes = false): Response
public function redirectAction(Request $request, string $route, bool $permanent = false, $ignoreAttributes = false, bool $keepRequestMethod = false): Response

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Feb 18, 2018

Member

for other reviewers: the class is final, so this is not a BC break

{
if ('' == $route) {
throw new HttpException($permanent ? 410 : 404);
throw new HttpException($permanent ? Response::HTTP_GONE : Response::HTTP_NOT_FOUND);

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Feb 18, 2018

Member

we prefer explicit numbers; please revert, and use numbers everywhere in the PR

This comment has been minimized.

@ZipoKing

ZipoKing Feb 18, 2018

Contributor

@nicolas-grekas Change reverted

@nicolas-grekas nicolas-grekas added this to the 4.1 milestone Feb 18, 2018

@fabpot

fabpot approved these changes Feb 19, 2018

@fabpot

This comment has been minimized.

Member

fabpot commented Feb 19, 2018

Thank you @ZipoKing.

@fabpot fabpot closed this Feb 19, 2018

fabpot added a commit that referenced this pull request Feb 19, 2018

feature #26213 [FrameworkBundle] Add support to 307/308 HTTP status c…
…odes in RedirectController (ZipoKing)

This PR was squashed before being merged into the 4.1-dev branch (closes #26213).

Discussion
----------

[FrameworkBundle] Add support to 307/308 HTTP status codes in RedirectController

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |  #26171
| License       | MIT
| Doc PR        |

With this PR `RedirectController` will allow to create redirections with use of 307/308 HTTP status codes together with 301/302. Related RFC documents:
* https://tools.ietf.org/html/rfc7231
* https://tools.ietf.org/html/rfc7538

Commits
-------

64fb5a5 [FrameworkBundle] Add support to 307/308 HTTP status codes in RedirectController
@stof

This comment has been minimized.

Member

stof commented Feb 19, 2018

this needs a documentation PR though

@ZipoKing

This comment has been minimized.

Contributor

ZipoKing commented Feb 19, 2018

@stof will prepare some docs PR later today

javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Mar 5, 2018

minor #9298 [FrameworkBundle] PR #26213 Document redirections with 30…
…7/308 HTTP status codes (ZipoKing, javiereguiluz)

This PR was merged into the master branch.

Discussion
----------

[FrameworkBundle] PR #26213 Document redirections with 307/308 HTTP status codes

This documents changes introduced with pull request symfony/symfony#26213

Commits
-------

9468bf9 Reword and added an example
c8ce65d Changes after @Simperfit  review
b3984d0 [PR #26213 Document redirections with 307/308 HTTP status codes

@fabpot fabpot referenced this pull request May 7, 2018

Merged

Release v4.1.0-BETA1 #27181

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment