-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
content-disposition header case sensitivity #1540
Conversation
According to RFC 6266, it should always be |
Ideally yes, I would suggest keeping it flexible. |
@@ -30,6 +30,7 @@ class FormData { | |||
/// Whether [finalize] has been called. | |||
bool get isFinalized => _isFinalized; | |||
bool _isFinalized = false; | |||
bool camelCaseContentDisposition = false; |
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 would be better to be a final field, and initialize it from constructors.
@@ -41,6 +42,11 @@ class FormData { | |||
ListFormat collectionFormat = ListFormat.multi, | |||
]) { | |||
_init(); | |||
|
|||
if (map.containsKey("camelCaseContentDisposition")) { |
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.
Using an argument might be better.
Update the PR, |
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.
I don't think your recent changes to the code can pass static analysis. Though I'll try to merge this into the fork without extra steps from your side.
dio/lib/src/form_data.dart
Outdated
|
||
FormData() { | ||
FormData({bool camelCaseContentDisposition = false}) { | ||
_camelCaseContentDisposition = camelCaseContentDisposition; |
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.
This is invalid since _camelCaseContentDisposition
is final
.
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.
updated
dio/lib/src/form_data.dart
Outdated
ListFormat collectionFormat = ListFormat.multi, | ||
{bool camelCaseContentDisposition = false} |
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.
This is impossible when positioned optional arguments are set.
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.
you are right, my mistake
- kept the param in the map for now
Hi @MrSagarShah. We've made our hardfork repo public and published a new version of |
@AlexV525 Thanks, Mate. |
New Pull Request Checklist
develop
to avoid conflicts (via merge from master or rebase)This merge request fixes / refers to the following issues: ...
Pull Request Description
...