HEAD request with 301 response results in GET request when following redirect

[webignition]

Q	A
Bug?	yes
New Feature?	no
Version	6.3.2

Actual Behavior

• send HEAD request
• receive 301 redirect response
• follow redirect with GET request

Expected Behavior

• send HEAD request
• receive 301 redirect response
• follow redirect with HEAD request

cURL will follow a redirect response to a HEAD request with another HEAD request. I think it is fair to assume that if cURL behaves in this manner then so should guzzle.

Steps to Reproduce

<?php
use GuzzleHttp\Client;
use GuzzleHttp\HandlerStack;
use GuzzleHttp\Middleware;
use GuzzleHttp\Psr7\Request;

$historyContainer = [];

$handlerStack = HandlerStack::create();
$handlerStack->push(Middleware::history($historyContainer));

$client = new Client(['handler' => $handlerStack]);

// Will 301 redirect to either https://www.google.com 
// or https://www.google.[tld for your country]
$request = new Request('HEAD', 'https://google.com/');

$client->send($request);

foreach ($historyContainer as $httpTransaction) {
    $request = $httpTransaction['request'];
    var_dump($request->getMethod() . ' ' . (string)$request->getUri());
}

// current dumped output:
// string(24) "HEAD https://google.com/"
// string(71) "GET https://www.google.co.uk/"

// ideal dumped output:
// string(24) "HEAD https://google.com/"
// string(71) "HEAD https://www.google.co.uk/"

Possible Solutions

Overview
RedirectMiddleware::modifyRequest() changes the request method to GET in cases where the response status code is 301 and where the request has a body (assuming non-strict redirects as is the default). The current code checks this as $statusCode <= 302 && $request->getBody()

I assume the checking for a body relates to the need to change from a non-safe method (such as POST) to a safe method (such as GET) and in this respect the method change makes sense.

A request can easily be created without a body, however GuzzleHttp\Psr7\Request::getBody() will always return a Psr\Http\Message\StreamInterface instance and therefore $request->getBody() will always evaluate to true.

Possible solution 1
Allow RedirectMiddleware::modifyRequest() to check if a request has a body without having to read the body. Not reading the body avoids all of the possible failure points that you might otherwise encounter when reading the body.

• add new method GuzzleHttp\Psr7\Request::hasBody() returning true or false as needed depending on the emptiness of GuzzleHttp\Psr7\MessageTrait::$stream
• updated RedirectMiddleware::modifyRequest() as needed to check if a request has a body

pros: will work
cons: introduces non-PSR7 method

Possible solution 2
Read the request body to a string RedirectMiddleware::modifyRequest() and modify the request method if the string is not empty.

pros: will work, doesn't introduce non-PSR7 method
cons: failure to read the request body

[webignition]

I'm going to create a PR for solution 2 as this is the least-effort solution and may well do the trick.

[brbrowning21]

On HTTP 301, RFC 7231 says:

Note: For historical reasons, a user agent MAY change the request method from POST to GET for the subsequent request. If this behavior is undesired, the 307 (Temporary Redirect) status code can be used instead.

To deal with 301, I think a boolean flag could be a good control for whether or not to coerce the subsequent request to a GET. If the response is a 307, then that flag could be bypassed, forcing the subsequent request to be the same method.

[webignition]

In PR #2047 I've opted for coercing the subsequent request to a GET if the original request method is not one that is safe i.e. retain the original request method if it is GET, HEAD or OPTIONS.

This negates the need to care about whether the original request has a body.