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

json content-type with charset not supported by some servers #258

Closed
dillon-giacoppo opened this issue Jul 30, 2019 · 5 comments
Closed
Assignees

Comments

@dillon-giacoppo
Copy link

Hello! I am using this to write a client for Artifactory. It looks like they do naive content-type parsing and do not accept charset=utf-8 in "application/json; charset=utf-8".

Section 6 of the application/json RFC states the media type has no optional or required parameters. Therefore whilst the Artifactory API might not be very robust, it is technically RFC compliant. The RFC also notes in section 3 that the charset of the json media type can be autodetected so there is not need to specify utf-8, utf-16, or utf-32.

Changing the media type to just application/json should in theory make the client compatible with more servers. There was an open issue with them to JFrog to add support for more robust parsing but it doesn't look like it has been touched in a few years https://www.jfrog.com/jira/browse/RTFACT-14981 and as mentioned before it is technically RFC compliant.

@jeevatkm jeevatkm self-assigned this Jul 31, 2019
@jeevatkm
Copy link
Member

@dillon-giacoppo Typically if you set the Content-Type header value in the request, Resty will not override user provided value.

// Same could be set at client level too
client.R().SetHeader("Content-Type", "application/json")

Can you please try and let me know?

@dillon-giacoppo
Copy link
Author

I have already worked around the issue with a pre-request hook that converts the header if it exists so I don’t have to include it on every request. It does however seem from looking into this, that the shorter form will have better compatibility with servers that do naive string matching and is RFC compliant while adding the charset parameter is not, so it may be a worthwhile change regardless

@jeevatkm jeevatkm added this to the v2.1.0 Milestone milestone Sep 8, 2019
@jeevatkm
Copy link
Member

jeevatkm commented Sep 8, 2019

@dillon-giacoppo Okay, I will update the JSON content-type without charset in the next release.

@jeevatkm
Copy link
Member

jeevatkm commented Sep 8, 2019

@dillon-giacoppo I have done changes in branch json-content-type-no-charset can you try it out and let me know?

@jeevatkm
Copy link
Member

It has been merged to master and scheduled for next release v2.1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants