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

the request body couldn't be sent correctly if content set to 'application/json; charset=utf-8' #90

Closed
lopaz opened this issue Oct 12, 2020 · 5 comments
Labels

Comments

@lopaz
Copy link

lopaz commented Oct 12, 2020

the following code doesn't work:
wretch().content('application/json; charset=utf-8').url('http://...').post(json)

it seems that charset=utf-8 is not allowed:

wretch/src/wretcher.ts

Lines 199 to 206 in 2631890

typeof body === "object" && (
!headers ||
Object.entries(headers).every(([k, v]) =>
k.toLowerCase() !== CONTENT_TYPE_HEADER.toLowerCase() ||
v === JSON_MIME
)
) ? this.json(body) :
this.body(body)

wretch/src/wretcher.ts

Lines 12 to 13 in 2631890

const JSON_MIME = "application/json"
const CONTENT_TYPE_HEADER = "Content-Type"

not sure if this is a bug, since json is the default content-type and utf-8 is the default chareset for json, which means content('application/json; charset=utf-8') is totally redundant.

@elbywan
Copy link
Owner

elbywan commented Oct 12, 2020

Hi @lopaz, thanks for reporting the issue!

not sure if this is a bug, since json is the default content-type and utf-8 is the default chareset for json, which means content('application/json; charset=utf-8') is totally redundant.

I think this is a bug, wretch should never override a Content-Type header having a custom charset. I'll push a fix to prevent this behaviour!

With that said, in theory according to RFC-4627 for the application/json content type the encoding is determined by the first four bytes of the payload:

JSON text SHALL be encoded in Unicode.  The default encoding is UTF-8.

Since the first two characters of a JSON text will always be ASCII
characters [RFC0020], it is possible to determine whether an octet
stream is UTF-8, UTF-16 (BE or LE), or UTF-32 (BE or LE) by looking
at the pattern of nulls in the first four octets.

I also noticed that browsers often override the content-type value regardless of the fetch options. So in practice, in that context omitting the charset should not matter since browsers add the value themselves.

@elbywan elbywan added the bug label Oct 12, 2020
@elbywan
Copy link
Owner

elbywan commented Oct 12, 2020

I just released version 1.7.3 with the fix 📦. Feel free to reopen if I missed something 😉.

@lopaz
Copy link
Author

lopaz commented Oct 14, 2020

@elbywan Good job. I didn't realize the charset-overwritten issue.

There's still another one, using wretch().content('application/json; charset=utf-8').url('http://...').post({foo: 'bar'}), the request payload would be [object Object] instead of {foo: 'bar'}, which made the request failed.
image

@elbywan
Copy link
Owner

elbywan commented Oct 14, 2020

@lopaz Good catch! I pushed a commit (af48887) that should solve the problem.

@lopaz
Copy link
Author

lopaz commented Oct 15, 2020

Confirmed, fixed in 1.7.4 👍

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

No branches or pull requests

2 participants