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

Add authentication args length check #1335

Merged
merged 5 commits into from Nov 15, 2021

Conversation

LarsStegman
Copy link
Contributor

@LarsStegman LarsStegman commented Nov 10, 2021

Fix for IndexError when both args and kwargs are specified, resulting in the possibility of trying to access a tuple index that is higher than the tuple size.

@LarsStegman
Copy link
Contributor Author

Did someone get a chance to take a look at this and run the workflows?

@Kludex
Copy link
Sponsor Member

Kludex commented Nov 15, 2021

Did someone get a chance to take a look at this and run the workflows?

Yes, but this approach looks more as a workaround to me. I've developed an alternative solution on #1337, which I'm unsure if we should merge it.

Copy link
Sponsor Member

@Kludex Kludex left a comment

Choose a reason for hiding this comment

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

I'm willing to accept it now. Just a linter issue :)

starlette/authentication.py Show resolved Hide resolved
starlette/authentication.py Outdated Show resolved Hide resolved
starlette/authentication.py Outdated Show resolved Hide resolved
@LarsStegman
Copy link
Contributor Author

Yes, I shouldn't try to fix it in the browser with no linting help haha. Thanks!

starlette/authentication.py Outdated Show resolved Hide resolved
Copy link
Sponsor Member

@Kludex Kludex left a comment

Choose a reason for hiding this comment

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

Thanks @LarsStegman ! 🎉

@Kludex Kludex merged commit 649161c into encode:master Nov 15, 2021
@LarsStegman
Copy link
Contributor Author

LarsStegman commented Nov 16, 2021

Could this fix be released in a new patch version so we can change our dependencies?

@LarsStegman LarsStegman changed the title Add authentication requires args length check Add authentication args length check Nov 16, 2021
@LarsStegman LarsStegman deleted the bug-fix/auth-args-idx branch November 17, 2021 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Authentication middleware IndexError on instance method
2 participants