Bug in request header validation about yin HOT 5 CLOSED

Hey, thank you for the report and for the suggestions for the fix! However, I think the current algorithm conforms to the specification. If you re-read the "Server Responsibilities" section in the spec (http://jsonapi.org/format/#content-negotiation-servers), you'll find that a server MUST only:

• send the Content-Type header in each response
• validate the Accept request header if it doesn't contain any media type parameters
• validate the Content-Type request header if it doesn't contain any media type parameters

At least this is how I interpret the rules. So the return strpos($header, "application/vnd.api+json") !== false || $header === "application/vnd.api+json"; conditions ensure that if the application/vnd.api+json media type is present, it isn't modified by any parameter.

You can read a little bit about the topic here: json-api/json-api#605

from yin.

Ok i just checked the specs and the problem here goes further than i thought and the check here is insufficient:
The original check is this:
return strpos($header, "application/vnd.api+json") === false || $header === "application/vnd.api+json";

Valid example Headers against the check:

• Accept: application/vnd.api+json
  • Valid through $header === "application/vnd.api+json"
• Accept: application/vnd.api+json,text/html
  • Invalid because first check returns false (strpos returns 0 which is not false) and second check returns false as well.
• Accept: */*
  • Valid because the first check matches
• Accept: text/*;q=0.3, text/html;q=0.7, text/html;level=1, text/html;level=2;q=0.4, /;q=0.5
  • Valid because the first check matches
• Accept: application/vnd.api+json, text/*;q=0.3, text/html;q=0.7, text/html;level=1, text/html;level=2;q=0.4, /;q=0.5
  • Invalid because first check returns false

Invalid Examples:

• Accept: application/vnd.api+json;q=0.7
  • Invalid because first check returns false

So to sum it up, the check needs to check the following:

• application/vnd.api+json is not present
  • VALID
• if the application/vnd.api+json is present
  • No ";" follows which indicates parameters

So i think the line line could be:
return strpos($header, "application/vnd.api+json") === false || strpos(str_replace(' ','',$header), "application/vnd.api+json;") === false;
But maybe a cool regex would be better here.

from yin.

Sorry, I currently have a very hectic year end at work. :S So I will be back in a couple of days. :)

from yin.

@gfemorris I am back after a couple of weeks :)

Thank you for the thorough investigation! I managed to think over the problem and I committed the following solution based on your suggestions: d435e7a

I hope that you have some time to review it. If everything is OK then I'll release a patch with the fix for Yin v2.0 and v3.0 equally.

from yin.

I had to change the code again because I realised that the first condition can be omitted (right?) and the regex would fail in the following scenario: "application/vnd.api+json;". I hope that the validation logic is good now.

dfcd047

from yin.