8000 307 redirect for POST/PUT requests · Issue #26171 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

307 redirect for POST/PUT requests #26171

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.

Sign up for GitHub

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

Jump to bottom
Closed
cooldude77 opened this issue Feb 14, 2018 · 13 comments
Closed

307 redirect for POST/PUT requests #26171

cooldude77 opened this issue Feb 14, 2018 · 13 comments
Labels
Feature

Comments

@cooldude77
Copy link
cooldude77 commented Feb 14, 2018 •
edited
Loading
Q A
Bug report? no
Feature request? yes
BC Break report? unsure
RFC? unsure
Symfony version 2.8.33

I have a version v2 of an API which has majority routes going back to V1, except for a few .
For v2


v2_rest_save_adresses:
    pattern:  api/v2/addresses
    defaults:
        _controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController::redirectAction
        route: rest_save_adresses
        permanent: false

For V1


rest_save_adresses:
    pattern:  api/v1/addresses
    defaults: { _controller: XYZRestBundle:Address:create}
    methods: [POST]

The GET requests get redirected with 302 code successfully, while the post requests see an error like this

No route found for "GET /api/v1/addresses": Method Not Allowed (Allow: POST)

The class that does the routing has handling only for parameters 301 and 302.


    /**
     * Redirects to another route with the given name.
     *
     * The response status code is 302 if the permanent parameter is false (default),
     * and 301 if the redirection is permanent.
     *
     * In case the route name is empty, the status code will be 404 when permanent is false
     * and 410 otherwise.
     *
     * @param Request    $request          The request instance
     * @param string     $route            The route name to redirect to
     * @param bool       $permanent        Whether the redirection is permanent
     * @param bool|array $ignoreAttributes Whether to ignore attributes or an array of attributes to ignore
     *
     * @return Response A Response instance
     *
     * @throws HttpException In case the route name is empty
     */
    public function redirectAction(Request $request, $route, $permanent = false, $ignoreAttributes = false)
    {
        if ('' == $route) {
            throw new HttpException($permanent ? 410 : 404);
        }

        $attributes = array();
        if (false === $ignoreAttributes || is_array($ignoreAttributes)) {
            $attributes = $request->attributes->get('_route_params');
            unset($attributes['route'], $attributes['permanent'], $attributes['ignoreAttributes']);
            if ($ignoreAttributes) {
                $attributes = array_diff_key($attributes, array_flip($ignoreAttributes));
            }
        }

        return new RedirectResponse($this->container->get('router')->generate($route, $attributes, UrlGeneratorInterface::ABSOLUTE_URL), $permanent ? 301 : 302);
    }

This is the reason that a request that is post and that should be redirected with 307 status code is not redirected at all, leading to even POST requests getting transferred as GET.

I just changed line return new RedirectResponse($this->container->get('router')->generate($route, $attributes, UrlGeneratorInterface::ABSOLUTE_URL), $permanent ? 301 : 302);

To
return new RedirectResponse($this->container->get('router')->generate($route, $attributes, UrlGeneratorInterface::ABSOLUTE_URL), 307);

And this works well ( of course not completely correct). I think this needs an enhancement to even allow POST requests to be redirected.

Proposed solution : (Since I have not learned the pull process completely)

    public function redirectAction(Request $request, $route, $permanent = false, $postOrPut = false, $ignoreAttributes = false)
    {
        if ('' == $route) {
            throw new HttpException($permanent ? 410 : 404);
        }

        $attributes = array();
        if (false === $ignoreAttributes || is_array($ignoreAttributes)) {
            $attributes = $request->attributes->get('_route_params');
            unset($attributes['route'], $attributes['permanent'], $attributes['ignoreAttributes']);
            if ($ignoreAttributes) {
                $attributes = array_diff_key($attributes, array_flip($ignoreAttributes));
            }
        }

        if (true == $postOrPut)
            $code = 307;
        else $code = $permanent ? 301 : 302;

        return new RedirectResponse($this->container->get('router')->generate($route, $attributes, UrlGeneratorInterface::ABSOLUTE_URL), $code);
    }

And in routing use this for POST```

v2_rest_save_adresses:
pattern: api/v2/addresses
defaults:
_controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController::redirectAction
route: rest_save_adresses
postOrPut: true

And for GET it is same as before
@Tobion
Copy link
Contributor
Tobion commented Feb 14, 2018 •
edited
Loading

I think the RedirectController can just automatically use 307 when the current scheme is not GET and permanent = false or 308 when permanent = true

@ZipoKing
Copy link
Contributor

@Tobion I don't think that it will work as I have met a lot of projects where e.g. resource save (POST) action at the end does redirect to show saved resource (GET) - with 307/308 redirect it will stop working. I think that it should be rather additional parameter for the redirect action which will say if it should be 301/302 or 307/308

@Tobion
Copy link
Contributor
Tobion commented Feb 17, 2018

@ZipoKing you wouldn't use the RedirectController for this. That would be strange.

@ZipoKing
Copy link
Contributor

Yes, I won't but as I said I have seen a lot of crazy code over my career. So therefore I'd rather go with extra bool param saying if we should use 301/302 (as default so backward compability will be kept) or 307/308. I'll try to make some PR later today about that.
8000

@soullivaneuh
Copy link
Contributor

@ZipoKing In this case, maybe put an option for 307/308 redirect with a deprecation message telling it will be the default on next major?

@ZipoKing
Copy link
Contributor

Pull request created: symfony/framework-bundle#22

@ZipoKing
Copy link
Contributor

(One minute, will create proper one for symfony/symfony)

@ZipoKing ZipoKing mentioned this issue Feb 18, 2018
Closed
@fabpot fabpot closed this as completed Feb 19, 2018
fabpot added a commit that referenced this issue Feb 19, 2018
@fabpot
feature #26213 [FrameworkBundle] Add support to 307/308 HTTP status c…
3aa59b6 
…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
@cooldude77
Copy link
Author

Thanks for adding it to 4.0.
Is it going to be added to 2.8 too , as the problem exists over there too ?

@Tobion
Copy link
Contributor
Tobion commented Feb 21, 2018

As this is a new feature, it's only available in upcoming versions.

@cooldude77
Copy link
Author
cooldude77 commented Feb 21, 2018 •
edited
Loading

@Tobion Thanks for your reply. However I believe it is more of a requirement not satisfied, and not a feature only , as there are definitely people, using 2.8 + or 3.+, who would use POST/PUT requests in the route. So it does have a problem with backward compatibility too.
8000

@nicolas-grekas
Copy link
Member

I confirm this is a new feature: that's how the redirect controller behaved for now. If you need the new behavior right now, you can copy/paste the new code in a controller of your own.

@cooldude77
Copy link
Author

Alright @nicolas-grekas , Thanks for your reply :)

@valentin-harrang
Copy link

Is it possible to send POST parameters to a URL via a RedirectResponse 307 where the "name" of an HTML field is the name of a parameter of the array in the RedirectResponse (for example : "NOM" is the "name" of an HTML field)?

I tried this but the server doesn't seem to receive my params :

return new RedirectResponse('https://myurl.com', 307, [
    'NOM'        => $utilisateur->getNom(),
    'PRENOM'     => $utilisateur->getPrenom(),
    'TEL'        => $utilisateur->getTel(),
    'EMAIL'      => $utilisateur->getEmail(),
    'PAYS'       => $utilisateur->getPays(),
]);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@fabpot @javiereguiluz @nicolas-grekas @Tobion @ZipoKing @soullivaneuh @cooldude77 @valentin-harrang
0