-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Use 308 to ensure http method is preserved #9286
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,7 @@ trailing slash to the same URL without a trailing slash | |
|
||
Create a controller that will match any URL with a trailing slash, remove | ||
the trailing slash (keeping query parameters if any) and redirect to the | ||
new URL with a 301 response status code:: | ||
new URL with a 308 (*HTTP Permanent Redirect*) response status code:: | ||
|
||
// src/AppBundle/Controller/RedirectingController.php | ||
namespace AppBundle\Controller; | ||
|
@@ -27,7 +27,9 @@ new URL with a 301 response status code:: | |
|
||
$url = str_replace($pathInfo, rtrim($pathInfo, ' /'), $requestUri); | ||
|
||
return $this->redirect($url, 301); | ||
// 308 (Permanent Redirect) is similar to 301 (Moved Permanently) except | ||
// that it does not allow changing the request method from POST to GET | ||
return $this->redirect($url, 308); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should not be 301 if it's a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, why? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See https://httpstatuses.com/308, it works for both There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 Then. But not all people know what |
||
} | ||
} | ||
|
||
|
@@ -50,7 +52,7 @@ system, as explained below: | |
{ | ||
/** | ||
* @Route("/{url}", name="remove_trailing_slash", | ||
* requirements={"url" = ".*\/$"}, methods={"GET"}) | ||
* requirements={"url" = ".*\/$"}) | ||
*/ | ||
public function removeTrailingSlashAction(Request $request) | ||
{ | ||
|
@@ -65,7 +67,6 @@ system, as explained below: | |
defaults: { _controller: AppBundle:Redirecting:removeTrailingSlash } | ||
requirements: | ||
url: .*/$ | ||
methods: [GET] | ||
|
||
.. code-block:: xml | ||
|
||
|
@@ -92,20 +93,10 @@ system, as explained below: | |
), | ||
array( | ||
'url' => '.*/$', | ||
), | ||
array(), | ||
'', | ||
array(), | ||
array('GET') | ||
) | ||
) | ||
); | ||
|
||
.. note:: | ||
|
||
Redirecting a POST request does not work well in old browsers. A 302 | ||
on a POST request would send a GET request after the redirection for legacy | ||
reasons. For that reason, the route here only matches GET requests. | ||
|
||
.. caution:: | ||
|
||
Make sure to include this route in your routing configuration at the | ||
|
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.
Not related, but we can take benefit to fix it: why a space on
rtrim
second argument?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.
Because you want to remove spaces too
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.
Why? I tried without, works well.
Uh oh!
There was an error while loading. Please reload this page.
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.
I mean what happens if the url is
http://my.website/someroute/ <- space here
? That works too?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.
@greg0ire Yes.
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.
But I did it on Chrome, maybe the browser remove it itself? 🤔
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.
Maybe, because I can't see how that would work