-
-
Notifications
You must be signed in to change notification settings - Fork 536
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
Disable multipart uploads by default #3645
Disable multipart uploads by default #3645
Conversation
Reviewer's Guide by SourceryThis pull request disables multipart uploads by default and adjusts the Django view to no longer be implicitly exempted from Django's built-in CSRF protection. These changes are aimed at improving security by making users explicitly opt-in to features that may have security implications. File-Level Changes
Sequence DiagramsequenceDiagram
participant C as Client
participant V as Strawberry View
participant H as HTTP Handler
C->>V: HTTP Request
V->>H: Parse HTTP Body
alt multipart_uploads_enabled is True
H->>H: Process Multipart Data
else multipart_uploads_enabled is False
H->>H: Reject Multipart Data
end
H->>V: Parsed Data
V->>C: HTTP Response
Tips
|
for more information, see https://pre-commit.ci
Thanks for adding the Here's a preview of the changelog: Starting with this release, multipart uploads are disabled by default and Strawberry Django view is no longer implicitly exempted from Django's CSRF protection. These are breaking changes if you are using multipart uploads OR the Strawberry Django view. Here's the tweet text:
|
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.
Hey @DoctorJohn - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟡 Security: 1 issue found
- 🟡 Testing: 2 issues found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3645 +/- ##
===========================================
+ Coverage 72.55% 96.76% +24.21%
===========================================
Files 518 522 +4
Lines 32647 33824 +1177
Branches 3772 5635 +1863
===========================================
+ Hits 23687 32731 +9044
+ Misses 8532 863 -7669
+ Partials 428 230 -198 |
CodSpeed Performance ReportMerging #3645 will not alter performanceComparing Summary
|
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.
adc8fa5
to
4339058
Compare
4339058
to
f5d9b0b
Compare
Description
This PR disables support for the GraphQL multipart request spec (i.e, multipart uploads) by default and adjusts the Django view to no longer be implicitly exempted from Django's built-in CSRF protection.
These are breaking changes for those using multipart uploads AND/OR the Django view integration.
Types of Changes
Summary by Sourcery
Disable multipart uploads by default and remove implicit CSRF exemption for Django views, requiring users to opt-in for these features. Update documentation and tests to reflect these changes and add release notes for the breaking changes.
Enhancements:
Documentation:
Tests:
Chores: