-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle] Add support to 307/308 HTTP status codes in RedirectController #26213
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we prefer explicit numbers; please revert, and use numbers everywhere in the PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nicolas-grekas Change reverted
Thank you @ZipoKing. |
…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
this needs a documentation PR though |
@stof will prepare some docs PR later today |
…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
With this PR
RedirectController
will allow to create redirections with use of 307/308 HTTP status codes together with 301/302. Related RFC documents: