-
Notifications
You must be signed in to change notification settings - Fork 18
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
Remove flask-featureflags integration, bump flask to v0.12.4 #447
Conversation
3cfd7a3
to
b89b8ed
Compare
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.
Nice
dmutils/errors.py
Outdated
# This is the remnant of a workaround for a bug in Flask 0.10.1. In that case, CSRFErrors | ||
# are caught under 400 BadRequest exceptions, so this handler can be registered to catch | ||
# all 400s, but will immediately discards non-CSRFError instances. Can be removed once | ||
# we're absolutely certain nothing is still using Flask 0.10.1. |
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.
I think we can remove it now. If the app has pulled in this version of utils it will have Flask 0.12, and flask_init
will only supply this function with recognisable CSRFError
instances (as opposed to, say, BadRequest
).
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.
For clarity we could rename the e
arg to csrf_error
?
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.
👍
b89b8ed
to
c35580e
Compare
734d7e8
to
321abe2
Compare
…dler for CSRFError we can do this because of the upgrade to flask 0.12 fixing the bug we were working around here.
https://trello.com/c/ZH9trgjH/419-snyk-alert-for-flask-010x-vulnerability
This broadly does three things:
CSRFError
handling workaround required for flask 0.10.x's deficiency, instead registering the handler in a more obvious way.Making an upgrade to this version will require bumping the app's Flask to 0.12.4, which itself can require some changes. Luckily I've already done this for all (?) the apps and PRs should be following for them. I'll probably only merge this once people are happy with them too.