search

php-http / client-common

Common HTTP Client implementations and tools for HTTPlug
http://httplug.io
MIT License
1.03k stars 54 forks source link

Add an option for RedirectPlugin not to modify request on statuses 300, 301, 302 #207

Closed olexiyk closed 3 years ago

olexiyk commented 3 years ago

Description I doubted if I need to create a bug ticket, but then as I also will follow up with a solution that in backwards compatible, I would like to post the issue as a feature request

Currently statuses 300, 301, 302 are switching from methods that are not GET and HEAD to GET, even though specification states that it's not allowed, but at the same time, it is a common implementation by http clients and browsers

https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3

If the 302 status code is received in response to a request other than GET or HEAD, the user agent MUST NOT automatically redirect the request unless it can be confirmed by the user, since this might change the conditions under which the request was issued.

Note: RFC 1945 and RFC 2068 specify that the client is not allowed to change the method on the redirected request. However, most existing user agent implementations treat 302 as if it were a 303 response, performing a GET on the Location field-value regardless of the original request method. The status codes 303 and 307 have been added for servers that wish to make unambiguously clear which kind of reaction is expected of the client.

So, according to specs that should happen

Request 1. POST http://example.com/api Response 1. 302 Location https://example.com/api Request 2. POST https://example.com/api

Currently that happens with client-common's RedirectPlugin

Request 1. POST http://example.com/api Response 1. 302 Location https://example.com/api Request 2. GET https://example.com/api

My suggestion would be to add strict option with default value false to RedirectPlugin a very similar way as guzzle's RedirectMiddleware does that:

https://github.com/guzzle/guzzle/blob/de6f1e58e735754b888649495ed4cb9ae3b19589/src/RedirectMiddleware.php#L157-L169

        // Use a GET request if this is an entity enclosing request and we are
        // not forcing RFC compliance, but rather emulating what all browsers
        // would do.
        $statusCode = $response->getStatusCode();
        if ($statusCode == 303 ||
            ($statusCode <= 302 && !$options['allow_redirects']['strict'])
        ) {
            $safeMethods = ['GET', 'HEAD', 'OPTIONS'];
            $requestMethod = $request->getMethod();

            $modify['method'] = in_array($requestMethod, $safeMethods) ? $requestMethod : 'GET';
            $modify['body'] = '';
        }

Additional context

Even if the specification requires the method (and the body) not to be altered when the redirection is performed, not all user-agents align with it - you can still find this type of bugged software out there. It is therefore recommended to use the 301 code only as a response for GET or HEAD methods and to use the 308 Permanent Redirect for POST methods instead, as the method change is explicitly prohibited with this status.

Even if the specification requires the method (and the body) not to be altered when the redirection is performed, not all user-agents conform here - you can still find this type of bugged software out there. It is therefore recommended to set the 302 code only as a response for GET or HEAD methods and to use 307 Temporary Redirect instead, as the method change is explicitly prohibited in that case.

In the cases where you want the method used to be changed to GET, use 303 See Other instead. This is useful when you want to give a response to a PUT method that is not the uploaded resource but a confirmation message such as: 'you successfully uploaded XYZ'.