-
Notifications
You must be signed in to change notification settings - Fork 971
feat: set default content type when processing body #825
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
base: main
Are you sure you want to change the base?
Conversation
httparty used to properly set multipart content type when dealing with files or multipart url bodies. There was no default for urlencoded. net-http 0.7.0 stops defaulting content type to x-www-form-urlencoded (net-http PR jnunemaker#207), thus httparty's default behavior of urlencoding needs to set content type. Content type does not get set when passed or when using custom query_string_normalizer. If you use custom query_string_normalizer, return content_type as a second argument.
| params.respond_to?(:to_hash) && (force_multipart || has_file?(params)) | ||
| end | ||
|
|
||
| def content_type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it acts as a getter when set, and tries to calc when body is not yet evaluated.
| if multipart? | ||
| @content_type = "multipart/form-data; boundary=#{boundary}" | ||
| generate_multipart | ||
| else | ||
| normalize_query_and_set_content_type(params) | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useless change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cus generate_multipiart sets @content_type, too
|
Solved by #828? |
|
I don't think #828 is a good PR, as it tries to guess content type based on whether body responds to some method and whether or not it was already set. HTTParty decides what gets sent. You can't really send a hash through a network. Hash is a ruby object. You send a string of bytes. The encoding is in there somewhere, done by HTTParty. Why guess when you can be sure. Set it exactly when encoding. This is what this pr does. |
|
Can we get this merged? As it does actually seem like a better idea than #828 |
|
#828 already solved the problem with a minimal, non-breaking fix. This would be a breaking fix, correct? custom query_string_normalizer must now return [query, content_type] tuple? |
|
I meant it to be backwards compatible. It does not raise, but it assigns |
httparty does properly set multipart content type when dealing with files or multipart url bodies. There was no default for urlencoded.
net-http 0.7.0 stops defaulting content type to x-www-form-urlencoded (net-http PR #207, issue #205). HTTParty encodes body with urlencode by default, but does not set content-type. HTTParty's default behavior of urlencoding needs to set content type.
changes in this pr:
If you use custom query_string_normalizer, return content_type as a second argument or pass a
Content-Typeheader