-
Notifications
You must be signed in to change notification settings - Fork 382
Don't alter Content-Type in Request if user explicitly sets it (#184) #886
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
Conversation
CC @brianquinlan @devoncarew – let's be careful with any change here. This has been "the way it is" for a very long time. |
Yes, it's been this way for years, despite the original issue beeing labeld as http/pkgs/http/lib/src/request.dart Line 96 in 805a147
to something like } else if (!contentType.parameters.containsKey('charset') && appendCharsetIfNotSet) { |
I completely agree. This exact behaviour makes it impossible to use this package for API calls to a filemaker server because the server rejects the sneakily added extra header. |
It turns out that the automatic setting of a charset in Content-Type can be avoided by not setting the This was observed last year in the #184 thread, and documented in #1014. See docs: So that seems like a pretty reasonable way to avoid setting a charset. And conversely, it seems pretty appropriate that if you do choose to set Given that, I think it'd be appropriate to close this issue as resolved (by the documentation PR #1014), just as #184 was resolved. |
See notes from @gnprice – feel free to push back |
This discussion is in fact already a year old, but I would like to warm it up again: In fact, from a user perspective, the documentation is not very insightful in this case: /// If [body] is a `Map`, it's encoded as form fields using [encoding]. The
/// content-type of the request will be set to
/// `"application/x-www-form-urlencoded"`; this cannot be overridden. Hence, I don't understand why this issue has not yet been fixed. The most up-to-date and also non-breaking proposal to add an optional boolean to the
I would happily create a PR for that as this is a huge dealbreaker for this simple |
I'm using this library and came across a very frustrating bug, see #184.
The core problem is, that this library alters the Content-Type header I (as a developer) explicitly set.
This is dangerous behaviour and should be removed.