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

noData() sends invalid response #365

Closed
tsongas opened this issue Nov 30, 2017 · 9 comments
Closed

noData() sends invalid response #365

tsongas opened this issue Nov 30, 2017 · 9 comments

Comments

@tsongas
Copy link

tsongas commented Nov 30, 2017

Calling return noData() still sends an application/json content type header with the response, even though the body does not contain valid JSON. This causes a client that expects valid JSON to choke on the response. I am therefore unable to use the noData() function, and have to do this instead return representationOf({}) so that an empty response is still valid JSON. Maybe noData() should either send a valid empty JSON object, or the application/json header should not be sent.

@atuttle
Copy link
Owner

atuttle commented Nov 30, 2017

Interesting perspective, thanks for sharing. I've had the same problem and didn't think it was the header causing it but rather that the ajax library I was using as my REST client was not handling the response appropriately.

I suppose it does make some sense that when the response body is empty the Content-Type header should be something else. Any suggestion about what it should be? Perhaps text/plain ?

@ryanguill
Copy link
Collaborator

Would the right answer be to omit the content-type header entirely? or is that required?

@tsongas
Copy link
Author

tsongas commented Nov 30, 2017

I'm not sure about technical requirements but to me it's common sense that if there's no content, there should be no content-type header.

@tsongas
Copy link
Author

tsongas commented Dec 2, 2017

It might help to see how Frisby decides whether to automatically parse JSON https://github.com/vlucas/frisby/pull/412/files.

@adeleusiere
Copy link

adeleusiere commented Dec 20, 2017

I was also wondering about this (small) issue.

It is interesting how Frisby is dealing with this.
By default, noData() should also set the HTTP Status code to 204 (No Content, see https://httpstatuses.com/204). And I agree it makes sense to omit the Content-Type header.

Also, for the record, could be a good thing to change the function name from noData() to noContent(), with a backward compatibility of course. But then, there is the documentation and so on to be modified...

@atuttle
Copy link
Owner

atuttle commented Dec 21, 2017

All sounds good to me. PR's welcome. :)

And there is a process for docs PR's too. ;)

@adeleusiere
Copy link

adeleusiere commented Dec 22, 2017

Well, I think I have time for this one, I'll do it, will be my xmas gift ;-)
(you can assign to me)

@atuttle
Copy link
Owner

atuttle commented Dec 27, 2017

@adeleusiere can't assign to you without adding you as a committer, but we can let this comment stand in for assignment. Please consider it yours unless otherwise stated. :)

@adeleusiere
Copy link

Well... I made the change to have a 204 HTTP status code. But to omit the Content-Type header, it's not a piece of cake. I don't find a solution, even by looking at the underlying Tomcat ResponseFacade (https://tomcat.apache.org/tomcat-8.0-doc/api/index.html).

<cfscript> rs = getPageContext().getResponse(); dump(rs); </cfscript>

Is someone has an idea how to force that?

Otherwise, I would propose to set the Content-Type to text/plain to avoid misinterpretation of empty JSON string.

adeleusiere pushed a commit to adeleusiere/Taffy that referenced this issue Dec 28, 2017
atuttle added a commit that referenced this issue Oct 3, 2018
 noData() sends invalid response #365
@atuttle atuttle closed this as completed Oct 3, 2018
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

4 participants