middlewares/negotiation

missing accept-header

thepercival opened this issue · 5 comments

thepercival commented

Hello,

I noticed that the microsoft browser edge does not send an accept-header when doing an OPTIONS-request.
Postman or chrome does send the header { 'Accept': '/' } by default.

I use Slim Framework 4 like this:
$app->add((new Middlewares\ContentType(['html', 'json']))->errorResponse());

ContentType::detectFromHeader will not find a corresponding mime-type without the accept header.

I could add the accept-header before the ContentType-middleware is called when dealing with a OPTIONS-request, like this:
if( $request->getMethod() === 'OPTIONS' && $request->hasHeader('Accept') === false ) {
$request = $request->withHeader('Accept', '/');
}

But is this the way to handle this problem?

thanks for your effort!
Coen

oscarotero commented

Hi, thanks for bringing this up!
According to the specs (https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html):

If no Accept header field is present, then it is assumed that the client accepts all media types. If an Accept header field is present, and if the server cannot send a response which is acceptable according to the combined Accept field value, then the server SHOULD send a 406 (not acceptable) response.

So I think the good solution is returning the default type if no Accept header is found, and only return an errorResponse if no format has found but the header exists.

What do you think? Do you want to work on a pull request? It can be fixed easily here: https://github.com/middlewares/negotiation/blob/master/src/ContentType.php#L190

thepercival commented

yes, that is going to be my first pull request on an other persons project! I shall read the specs and try to make a pr.

oscarotero commented

There is always a first time for everything 😄
Thanks you!

oscarotero commented

Version 2.0.1 released. Thanks for your contribution!

thepercival commented
  • ❤️1