dart-lang / http_parser

A platform-independent Dart package for parsing and serializing HTTP formats.
https://pub.dev/packages/http_parser
BSD 3-Clause "New" or "Revised" License
39 stars 28 forks source link

add default charset of application/json to utf8 #51

Closed wferem1 closed 3 years ago

wferem1 commented 3 years ago

PR for dart-lang/http#1331

google-cla[bot] commented 3 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

wferem1 commented 3 years ago

@googlebot I signed it!

wferem1 commented 3 years ago

@kevmoo since the other PR dart-lang/http_parser#48 is not working, i created a new

kevmoo commented 3 years ago

We'll need tests here, too. CC @natebosch – I'm really not working

natebosch commented 3 years ago

I'm not sure if it makes sense to set a default in this library since it will be a behavior difference against the HTTP parsing that happens in the SDK. We're also looking at whether it makes sense to default to UTF8 decoding in packge:http when reading response.body as a string: https://github.com/dart-lang/http/issues/175

cc @lrhn - do you have any thoughts on the right place for this type of defaulting?

wferem1 commented 3 years ago

I added a test for the default charset. I understand that this may not seem the right place to do this defaulting, so you may close this PR if it is done in the http package.

lrhn commented 3 years ago

I don't think this is where I'd insert a default (if anywhere). Modifying the source data early in the process prevents later code from recognizing that there originally was no charset and do something customized in that case.

I'd rather do something at the point where the charset is consumed. That is, if we try to get an encoding for the body, and the media type has no charset parameter, then I'd see if there is a default encoding for that particular media type. If there isn't, I'm inclined to actually refuse to decode without an encoding being provided by the user, rather than defaulting to something (especially if that something is Latin-1, because that encoding is irrefutable, and will accept any incoming bytes whether intended as Latin-1 or not).

wferem1 commented 3 years ago

@lrhn I agree with that reasoning, I will close this PR, since this is already being discussed in dart-lang/http#175