-
-
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
Fix FormData clone boundary inconsistency issue #2305
Conversation
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.
Is it possible to directly update the _boundary
after the construction, so we don't need an extra argument for the constructor which won't increase the complexity for usages?
I apologize for the delayed response as I've been quite busy lately. Regarding your suggested solution - I want to make sure I understand it correctly. Would this be what you had in mind? FormData({
String? initBoundary,
this.boundaryName = _boundaryName,
this.camelCaseContentDisposition = false,
}) {
_init();
if(initBoundary != null) {
_boundary = initBoundary;
}
} |
No. What I mean is: // Convenience method to clone finalized FormData when retrying requests.
FormData clone() {
final clone = FormData();
clone._boundary = _boundary; // <-- Replace the boundary.
clone.fields.addAll(fields);
for (final file in files) {
clone.files.add(MapEntry(file.key, file.value.clone()));
}
return clone;
} |
Ah, I see. Your solution is indeed much better than what I proposed. Thank you for the guidance. I have submitted a new one implementing your suggested approach. Please review again, thanks. |
Please add the CHANGELOG entry and update the PR checklist status. |
…ix-clone-boundary-issue # Conflicts: # dio/CHANGELOG.md
done! |
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.
Thanks for the update. One last nit
Code Coverage Report: Only Changed Files listed
Minimum allowed coverage is |
Resolves #2303
Fix boundary inconsistency in FormData.clone method
Description:
This PR addresses an issue where cloning FormData objects resulted in different boundaries each time. The problem was caused by allocating a new boundary for each clone without any boundary reuse logic.
Changes made:
initBoundary
parameter to the FormData constructor.initBoundary
affectsboundaryName
.Technical details:
FormData
constructor now accepts an optionalinitBoundary
parameter.initBoundary
is provided, it overrides theboundaryName
configuration.Impact:
This fix ensures consistent behavior when cloning FormData objects and improves efficiency by reusing boundaries where possible. It should resolve issues related to unexpected boundary changes during FormData manipulation.
Testing:
Note to reviewers:
Please pay special attention to the boundary reuse logic and any potential side effects on existing FormData usage.
New Pull Request Checklist
main
branch to avoid conflicts (via merge from master or rebase)CHANGELOG.md
in the corresponding package