Skip to content

Add authentication args length check#1335

Merged
Kludex merged 5 commits intoKludex:masterfrom
LarsStegman:bug-fix/auth-args-idx
Nov 15, 2021
Merged

Add authentication args length check#1335
Kludex merged 5 commits intoKludex:masterfrom
LarsStegman:bug-fix/auth-args-idx

Conversation

@LarsStegman
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

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

@Kludex
Copy link
Copy Markdown
Owner

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
Copy Markdown
Owner

@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 :)

@LarsStegman
Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Owner

@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 Kludex:master Nov 15, 2021
@LarsStegman
Copy link
Copy Markdown
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