Skip to content
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

FastRoute falls back to GET on HEAD request #2386

Closed
garnold opened this issue Jan 24, 2018 · 9 comments
Closed

FastRoute falls back to GET on HEAD request #2386

garnold opened this issue Jan 24, 2018 · 9 comments

Comments

@garnold
Copy link

garnold commented Jan 24, 2018

We were trying to figure out why HEAD requests from bots were making their way into our controllers. Then we discovered that FastRoute falls back to GET on HEAD requests.

This is unexpected.

Related, I've noticed a number of issues where Slim users are requesting an $app->head(...) shortcut be added to Slim, and the response is often to use $app->map(['HEAD'], ...) instead. But given FastRoute's fallback you can just use $app->get(...) to handle HEAD requests. So my guess is this fallback behavior isn't well known in the Slim community.

@geggleto
Copy link
Member

Hmm, HEAD should be GET just w/o a response body.

@damianopetrungaro
Copy link

@akrabat
Copy link
Member

akrabat commented May 24, 2018

Do we correctly ditch the body when it's a HEAD?

@designermonkey
Copy link
Member

I'm not sure we do from a quick glance. We have the isEmpty method on a Response, but I can't see where we would check the Request to see if it is a HEAD request.

@akrabat
Copy link
Member

akrabat commented Sep 16, 2018

Todo:

  1. make sure that the user can detect a HEAD request inside the route callable that's attached to a GET request
  2. Ensure that we don't send the request body if it's a HEAD request.

@akrabat akrabat added this to the 4.0 milestone Oct 2, 2018
@akrabat
Copy link
Member

akrabat commented Dec 8, 2018

Addressed in #2547

@akrabat akrabat closed this as completed Dec 8, 2018
@akrabat akrabat reopened this Dec 8, 2018
@akrabat
Copy link
Member

akrabat commented Dec 8, 2018

Re-opening - 3.x needs checking.

@lordrhodos
Copy link

lordrhodos commented Feb 6, 2019

I did some research for the 3.x code.

Todo:

  1. make sure that the user can detect a HEAD request inside the route callable that's attached to a GET request

As the PSR-7 request is passed to the callable it is no problem to check manually if this is actually a HEAD request, e.g.

$app->get('/', function (Request $request, Response $response, array $args) {
...
    $isHeadMethod = $request->getMethod() === 'HEAD';
...
});
  1. Ensure that we don't send the request body if it's a HEAD request.

This is kind of tricky. The mentioned Response::isEmpty method just checks for a response status code that should not contain a body:

Slim/Slim/Http/Response.php

Lines 376 to 382 in 715d130

public function isEmpty()
{
return in_array(
$this->getStatusCode(),
[StatusCode::HTTP_NO_CONTENT, StatusCode::HTTP_RESET_CONTENT, StatusCode::HTTP_NOT_MODIFIED]
);
}

I think we need to integrate this at Slim\App::isEmptyResponse and add a check if we are dealing with a HEAD request, which will leave us with another fix that we need to attend here:

Slim/Slim/App.php

Lines 617 to 619 in 715d130

if ($this->isEmptyResponse($response)) {
return $response->withoutHeader('Content-Type')->withoutHeader('Content-Length');
}

Here we need to check if this is not a HEAD request before returning a response with removed headers.

@l0gicgate
Copy link
Member

Fixed with #2547 for Slim 4 and #2576 for Slim 3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants