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

Delete Response has false Content-Type #25

Closed
ghost opened this issue Jun 4, 2015 · 10 comments
Closed

Delete Response has false Content-Type #25

ghost opened this issue Jun 4, 2015 · 10 comments

Comments

@ghost
Copy link

ghost commented Jun 4, 2015

I'm doing some testing with supertest:

request(app)
        .delete '/some/' + someId.toString()
        .set headers
        .expect 'Content-Type', headers['Accept']
        .expect 204
        .end done

headers =
        'Content-Type' : 'application/vnd.api+json; charset=utf-8'
        'Accept' : 'application/vnd.api+json; charset=utf-8; supported-ext=bulk'

I get the following error:

Error: expected "Content-Type" of "application/vnd.api+json; charset=utf-8; supported-ext=bulk", got "application/vnd.api+json; supported-ext="bulk""

GET, POST and PATCH work very well

@ethanresnick
Copy link
Owner

Hi @benno208,

Thanks for reporting this! I just pushed a new version of the library that shouldn't have this problem; can you check?

Here's how it should behave now. First, the server should no longer send, and clients no longer need to indicate that they'll Accept, the supported-ext parameter. This is because extensions have now been marked as experimental and, in fact, the whole extension negotiation system may well change. So, until the extension system is more solid, I've chosen not to support the ext and supported-ext at all.

As for the charset parameter: if a client includes it in the Accept header, JSON API requires that the server respond with a 406 Not Acceptable. The idea is that, by requiring an explicit error, JSON API can add new parameters in the future (like ext) to the base spec, and clients can detect server support for those new parameters by seeing whether they get a 406.

The story with charset in Content-Type is different, though. First, express adds charset automatically, so there's little I can do about that. This is bad behavior on express's part, since JSON API's media type doesn't define a charset parameter, but it's not technically a violation of the media type spec. Nevertheless, if a client encounters charset in Content-Type, it MUST simply ignore it. This comes from JSON API but also from the media type spec, which says that "MIME implementations must ignore any parameters whose names they do not recognize".

So, in the end, here's what this adds up to:

  1. Your test client's Accept header should be simply application/vnd.api+json, since supported-ext is gone and including charset will trigger the JSON-API-mandated 406.
  2. Your client's Content-Type header should simply be application/vnd.api+json as well, as mandated here.
  3. Then, your expect clause should be that the returned Content-Type will be application/vnd.api+json; charset=utf8, even though this doesn't match the Accept header. This is a bit annoying—and it's a consequence of express's insistence on adding the charset parameter automatically, which I'd like to figure out some way to disable. But it's perfectly valid under HTTP, which says that using negotiation is optional ("If...none of the available representations for the response have a media type that is listed as acceptable, the origin server can... disregard the header field by treating the response as if it is not subject to content negotiation"). Moreover, it's consistent with JSON API's restrictions as well.

@ghost
Copy link
Author

ghost commented Jun 9, 2015

First thing after updating to 2.3.0:

Cannot find module 'babel-core/polyfill'

@ethanresnick
Copy link
Owner

Weird.... I just did a fresh install and things appear to be running fine. Can you give me the stack trace for that error?

@ghost
Copy link
Author

ghost commented Jun 11, 2015

I did a complete install. Now everything seems to be fine. When I do a POST I get

expected "Content-Type" of "application/vnd.api+json; charset=utf-8", got "application/json; charset=utf-8"

and on PATCH:

Error: expected "Content-Type" of "application/vnd.api+json; charset=utf-8", got "application/vnd.api+json"

@ethanresnick
Copy link
Owner

What's the Accept header you're sending? For some reason, it's thinking that generic JSON should be used rather than the JSON API media type.

@ghost
Copy link
Author

ghost commented Jun 12, 2015

I'm sending the following headers:

headers =
  'Content-Type' : 'application/vnd.api+json'
  'Accept' : 'application/vnd.api+json'

And I'm testing for:

response_headers =
   'Content-Type' : 'application/vnd.api+json; charset=utf-8'

@ghost ghost closed this as completed Jun 12, 2015
@ghost
Copy link
Author

ghost commented Jun 12, 2015

Damn, I clicked the false button...

@ghost ghost reopened this Jun 12, 2015
@ethanresnick
Copy link
Owner

Very weird...what you posted earlier should have worked (and was working for me). However, I've just pushed a new version of the library that removes the automatically-appended charset parameter all together. So now you should be able to send a request with:

Content-Type: application/vnd.api+json
Accept : application/vnd.api+json

And get back

Content-Type: application/vnd.api+json

There's a test for this behavior and, on my machine at least, it's passing. Let me know if you're still experiencing problems after upgrading to the latest version :)

@ghost
Copy link
Author

ghost commented Jun 13, 2015

Now, everything works like expected. Thanks!

@ghost ghost closed this as completed Jun 13, 2015
@ethanresnick
Copy link
Owner

Awesome!

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant