@dinooo13 Looking good, but I still think the logic for 303 should use the default behavior (HEAD stays HEAD and GET stays GET).
The logic behind your code works great, I am not quite sure if we're introducing to much complexity by adding two new methods. I came up with a shorter version of your code, still the same logic:
private function onResponseRedirect(ResponseInterface $response, RequestInterface $request, Deferred $deferred, ClientRequestState $state) { // resolve location relative to last request URI $location = Uri::resolve($request->getUri(), $response->getHeaderLine('Location')); $request = $this->makeRedirectRequest($request, $location, $response->getStatusCode()); $this->progress('redirect', array($request)); if ($state->numRequests >= $this->maxRedirects) { throw new \RuntimeException('Maximum number of redirects (' . $this->maxRedirects . ') exceeded'); } return $this->next($request, $deferred, $state); }
The first code block shows the onResponseRedirect() method. I have added $response->getStatusCode() as a parameter to the makeRedirectRequest() function call. The status code is the only part we need, this way we don't have to pass the whole response.
private function makeRedirectRequest(RequestInterface $request, UriInterface $location, Int $statusCode) { $originalHost = $request->getUri()->getHost(); $request = $request->withoutHeader('Host'); if ($statusCode !== 307 || $statusCode !== 308) { $request = $request ->withoutHeader('Content-Type') ->withoutHeader('Content-Length'); } // Remove authorization if changing hostnames (but not if just changing ports or protocols). if ($location->getHost() !== $originalHost) { $request = $request->withoutHeader('Authorization'); } $body = null; if ($statusCode === 307 || $statusCode === 308) { $method = $request->getMethod(); $body = $request->getBody(); } else { // naïve approach.. $method = ($request->getMethod() === 'HEAD') ? 'HEAD' : 'GET'; } return new Request($method, $location, $request->getHeaders(), $body); }
The second block shows the makeRedirectRequest() method. As you can see I've added the status code as an additional argument to this function. The rest follows the same logic as your suggestion, just a little less code. The if's will only check for the 307 and 308 status codes and add the original method and body to the new request. Every other 3xx status code uses the default logic which is already implemented.
The only thing I don't know how to handle yet are streaming bodies. The original request would send the whole body before the redirect response arrives. This means the new request can't contain the same body (data is already sent).
What are your thoughts on this?