Skip to content

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

Merged
merged 3 commits into from
Feb 20, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 6 additions & 15 deletions routing/redirect_trailing_slash.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -27,7 +27,9 @@ new URL with a 301 response status code::

$url = str_replace($pathInfo, rtrim($pathInfo, ' /'), $requestUri);
Copy link
Contributor

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?

Copy link
Contributor Author

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

Copy link
Contributor

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.

Copy link
Contributor Author

@greg0ire greg0ire Feb 18, 2018

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@greg0ire Yes.

Copy link
Contributor

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? 🤔

Copy link
Contributor Author

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


return $this->redirect($url, 301);
// 308 (Permanent Redirect) is similar to 301 (Moved Permanently) except
// that it does not allow changing the request method (e.g. from POST to GET)
return $this->redirect($url, 308);
}
}

Expand All @@ -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)
{
Expand All @@ -65,7 +67,6 @@ system, as explained below:
defaults: { _controller: AppBundle:Redirecting:removeTrailingSlash }
requirements:
url: .*/$
methods: [GET]

.. code-block:: xml

Expand All @@ -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
Expand Down