Respect/Rest

Problem with Content negotiation - Status code

Opened this issue · 2 comments

I'm using the latest Respect/Rest and probably I found a problem with content negotiation.

<?php

require __DIR__.'/vendor/autoload.php';

$r = new Respect\Rest\Router;
$contentNegotiation = [
   'text/html' => 'var_dump',
];
$r->get('/*/*', function(){
   return ['respect' => 'Ola!']; 
})->accept($contentNegotiation); 

$r->run();

If I send a request with header 'Accept: text/html' it works but if I send 'Accept: application/json', the application returns http 405 status code instead of 406.

I watch the source code, and at first the correct header is setted

protected function notifyDeclined($requested, $provided, Request $request, $params)
{
        $this->negotiated = false;
        header('HTTP/1.1 406');
}

Next, the status code is overwritten

protected function informMethodNotAllowed(array $allowedMethods)
    {
        header('HTTP/1.1 405');

        if (!$allowedMethods) {
            return;
        }

        $this->informAllowedMethods($allowedMethods);
        $this->request->route = null;
    }

I hope fix it and send a PR soon, meanwhile the @Respect team could help me too about how do it in better way :)

☁️

Odd... I was sure we had unit tests for this and didn't we fix those messages to include the... uhm... well... message? @cloudson are you sure you are using the latest...

Is the 405 method not allowed triggered on GET? This is also odd....

header() does have a second arg which takes a boolean whether to overwrite and defaults to true, but I somehow don't think that is the problem here...

Hmmm I see there are several that don't have the message added like 400 404 406 @cloudson please add the default messages while you're at it.