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

Problem with Content negotiation - Status code #106

Open
filhodanuvem opened this issue Mar 11, 2014 · 2 comments
Open

Problem with Content negotiation - Status code #106

filhodanuvem opened this issue Mar 11, 2014 · 2 comments
Labels
Milestone

Comments

@filhodanuvem
Copy link
Contributor

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 :)

☁️

@nickl-
Copy link
Member

nickl- commented Apr 14, 2014

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...

@nickl-
Copy link
Member

nickl- commented Apr 14, 2014

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.

@augustohp augustohp added this to the 1.0 milestone May 13, 2016
@augustohp augustohp added the bug label May 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants