Skip to content
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

Avoid raising errors on malformed POST data #511

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

sevdog
Copy link
Contributor

@sevdog sevdog commented Oct 3, 2023

Proposed changes

The login_redirect context-processor relies on request.POST data to retrive the redirect field value.

If by any means the django app is serving a malformed multipart request (ie: Content-Type: multipart/form-data without a proper form-boundary) it will raise a multipart error. Since this context-processor is advised to be used in the default template configuration it will cause a 500 response (internal server error) due to the un-catched exception.

Types of changes

Please check the type of change your PR introduces:

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (PEP8, lint, formatting, renaming, etc)
  • Refactoring (no functional changes, no api changes)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Build related changes (build process, tests runner, etc)
  • Other (please describe):

Checklist

  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works

@codecov
Copy link

codecov bot commented Oct 3, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (9e7ef7e) 94.07% compared to head (36ac3e8) 94.11%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #511      +/-   ##
==========================================
+ Coverage   94.07%   94.11%   +0.04%     
==========================================
  Files          39       39              
  Lines        1131     1139       +8     
  Branches      133      133              
==========================================
+ Hits         1064     1072       +8     
  Misses         43       43              
  Partials       24       24              
Flag Coverage Δ
unittests 94.11% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
social_django/context_processors.py 72.72% <100.00%> (+3.76%) ⬆️
tests/test_context_processors.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -19,3 +19,15 @@ def test_login_redirect_unicode_quote(self):
"REDIRECT_QUERYSTRING": "next=profile/sj%C3%B3",
},
)

def test_login_redirect_malformed_post(self):
request = self.request_factory.post("/", data="no boundary", content_type="multipart/form-data")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs formatting, see https://results.pre-commit.ci/run/github/69119240/1696338661.I5RZqwrgRPmTEG5Or7dWkw (or enable editing of the PR and pre-commit.ci will do the fixes for you).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, forgot to setup pre-commit locally.

@nijel nijel merged commit 3d1333b into python-social-auth:master Oct 4, 2023
8 checks passed
@nijel
Copy link
Member

nijel commented Oct 4, 2023

Merged, thanks for your contribution!

@sevdog sevdog deleted the fix-potential-post-errors branch October 4, 2023 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants