Content-Type and Accept headers not really under domain validation about yin HOT 7 CLOSED

Hey @samijnih, As per my understanding of the spec, fuck it, and fuck this too are perfectly valid media types. Quoting the related parts of the specification:

Servers MUST respond with a 415 Unsupported Media Type status code if a request specifies the header Content-Type: application/vnd.api+json with any media type parameters other than profile.

Servers MUST respond with a 406 Not Acceptable status code if a request’s Accept header contains the JSON:API media type and all instances of that media type are modified with a media type parameter other than profile.

All this means is that servers are not obliged to return an error, unless the application/vnd.api+json is present in (at least) one of headers and it is modified with a parameter other than profile.

from yin.

Hi @kocsismate

Thanks for taking time.

First of all from my understanding of the spec, it's about Client Side Responsabilities to send Content-Type with the JSON API media type.

Clients MUST send all JSON:API data in request documents with the header Content-Type: application/vnd.api+json without any media type parameters.

I think it should be cool to get at least a 415 HTTP code in case you don't give application/vnd.api+json like in the screenshots above from my postman about the client request headers.

What do you think?

from yin.

I'm very confused because actually your regex does not let me just put Content-Type: application/vnd.api+json without specifying a profile.

I have done that in your method to get negotiate work with or without a profile.

protected function isValidMediaTypeHeader(string $headerName): bool
    {
        $header = $this->getHeaderLine($headerName);

        // The media type is modified by media type parameters
        $matches = [];
        $isMatching = preg_match("/^.*application\/vnd\.api\+json\s*(;\s*([A-Za-z0-9]+)\s*=.*?)?$/i", $header, $matches);

        return $isMatching === 1 || (isset($matches[1]) && strtolower($matches[1]) === "profile");
    }

The spec does not treat about returning a 415 http code in case you don't give any profile and so, your regexp does not allow me to just give the json api media type from the client side

from yin.

I think it should be cool to get at least a 415 HTTP code in case you don't give application/vnd.api+json like in the screenshots above from my postman about the client request headers.

You can implement this in your application, but doing so at the framework level would cause problems for anybody who wants to strictly follow the specification.

The spec does not treat about returning a 415 http code in case you don't give any profile and so, your regexp does not allow me to just give the json api media type from the client side

Unfortuantely, I don't exactly get what you wrote. Could you please elaborate? Anyway, what I can tell for sure is that application/vnd.api+json is a valid JSON:API content type. Just have a look at it in the tests: https://github.com/woohoolabs/yin/blob/master/tests/JsonApi/Negotiation/RequestValidatorTest.php#L275

from yin.

@kocsismate my point is if I call negotiate, I'm expecting an exception to be raised or a 415 response code if I don't give application/vnd.api+json for Content-Type like on my first screenshots

from yin.

my point is if I call negotiate, I'm expecting an exception to be raised or a 415 response code if I don't give application/vnd.api+json for Content-Type like on my first screenshots

Yeah, I see what you want, however this is against the specification. I remember that there used to be issues in the past when people wanted to explore a JSON:API directly from browsers which by default send the Accept: text/html. If I implemented your feature request, this would be impossible to do from now on.

However, as a workaround, you could provide your custom implementation for RequestInterface::validateAcceptHeader and RequestInterface::validateContentTypeHeader(), or it's also possible to even extend the RequestValidator class, and override its negotiate() method. But on the framework level, this change shouldn't be implemented in my opinion.

That being said, I'm closing this PR.

from yin.

It's good for me. I don't wanna break the official spec :) 👍

from yin.