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

Make header reading compliant with RFC7230, section 3.2.2 #270

Merged
merged 11 commits into from
Sep 10, 2019

Conversation

Croug
Copy link
Contributor

@Croug Croug commented Sep 5, 2019

RFC7230, section 3.2.2 specifies that headers can be joined by comma separating the values so a header like this Authorization: Bearer <JWT>, Basic <B64EncodedCreds> is valid

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@pep8speaks
Copy link

pep8speaks commented Sep 5, 2019

Hello @Croug! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-09-10 21:01:57 UTC

@vimalloc
Copy link
Owner

vimalloc commented Sep 5, 2019

I had no idea that was valid, thanks for the pull request! If you don't mind, could you throw a quick unit test around this this? Then I will get it merged and a new release cut ASAP.

Thanks!

@Croug
Copy link
Contributor Author

Croug commented Sep 6, 2019

Yeah no problem, I will get them set up tonight.

@coveralls
Copy link

coveralls commented Sep 6, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling cd33d54 on Croug:master into 391de47 on vimalloc:master.

@Croug
Copy link
Contributor Author

Croug commented Sep 6, 2019

I've added the unit tests

@Croug
Copy link
Contributor Author

Croug commented Sep 7, 2019

@vimalloc

Copy link
Owner

@vimalloc vimalloc left a comment

Choose a reason for hiding this comment

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

A couple minor things, if you could get those changed and fix the pep8 errors (#270 (comment)) I think this should be good to go.

Thanks for contributing!

flask_jwt_extended/config.py Outdated Show resolved Hide resolved
flask_jwt_extended/view_decorators.py Outdated Show resolved Hide resolved
@vimalloc
Copy link
Owner

Sorry for the delay in getting back to you, I was not around this weekend. I left a couple comments, if you can update those I think this looks great and I could get a new release cut.

Thanks!

@Croug Croug force-pushed the master branch 2 times, most recently from 2fc098e to 58850a8 Compare September 10, 2019 20:48
@Croug
Copy link
Contributor Author

Croug commented Sep 10, 2019

Alright, finally managed to fix all the pep8 issues, my editor was exhibiting some weird behavior that kept creating new issues, but it should all be good now, I also fixed the ambiguity call when multiple header fields have the same value and reverted the config change

@Croug Croug requested a review from vimalloc September 10, 2019 21:09
@vimalloc vimalloc merged commit cb988e1 into vimalloc:master Sep 10, 2019
@vimalloc
Copy link
Owner

Thanks! I'll get a new release cut stat.

@vimalloc
Copy link
Owner

Released as version 3.23.0. Thanks for contributing! 👍

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.

None yet

4 participants