-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
chore: Bump flask libs #22355
chore: Bump flask libs #22355
Conversation
Codecov Report
@@ Coverage Diff @@
## master #22355 +/- ##
=======================================
Coverage 67.00% 67.01%
=======================================
Files 1859 1859
Lines 71019 71037 +18
Branches 7766 7766
=======================================
+ Hits 47587 47605 +18
Misses 21410 21410
Partials 2022 2022
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@@ -626,7 +626,7 @@ def test_redirect_invalid(self): | |||
|
|||
self.login(username="admin") | |||
response = self.client.get(f"/r/{model_url.id}") | |||
assert response.headers["Location"] == "http://localhost/" | |||
assert response.headers["Location"] == "/" |
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.
Werkzeug 2.1.0 changelog:
Response.autocorrect_location_header is disabled by default. The Location header URL will remain relative, and exclude the scheme and domain, by default. pallets/werkzeug#2352
When Response.autocorrect_location_header was added in 2011 pallets/werkzeug@0ec643b, it was documented as "correct the location header to be RFC conformant". It was referring to RFC 2616. That was superseded by RFC 7231 in 2014, which allows relative URLs. MDN lists all browsers as compliant with this. Switch autocorrect_location_header to be disabled by default.
# get from cache | ||
rv = test_client.post(uri) | ||
rv = test_client.post(uri, json={}) |
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.
Werkzeug 2.1.0 changelog:
Request.get_json() will raise a 400 BadRequest error if the Content-Type header is not application/json. This makes a very common source of confusion more visible. pallets/werkzeug#2339
If there's no json param or json=None
then 400 http status will be thrown
Seeing that GHSA-7wxw-4483-3m34 is a disputed CVE, and only occurs when running Flask with the provided dev server, something you should never do on production for several reasons. regarding flask-caching I don't see on the CVE any submitted patches, GHSA-656c-6cxf-hvcv. This CVE is disputed also, since an attacker would have to be able to write to the cache itself, something that would cause severe issues all around not just by deserializing content with Pickle. Can you please just bump Pillow and resolve the current PR conflicts? Thank you |
# Conflicts: # tests/integration_tests/thumbnails_tests.py
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.
looks good, final comments
app.config.get("SQLALCHEMY_EXAMPLES_URI") | ||
or app.config["SQLALCHEMY_DATABASE_URI"] | ||
) | ||
backend = db_uri.split("://")[0] |
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.
this seems like an unrelated change, no?
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.
It's the related change. When you want to run the local tests with SQLALCHEMY_EXAMPLES_URI = None
you will have an exception here and I fix the issue.
URI to database storing the example data, points to
SQLALCHEMY_DATABASE_URI by default if set toNone
@@ -543,7 +543,7 @@ def test_get_samples_with_filters(test_client, login_as_admin, virtual_dataset): | |||
f"/datasource/samples?datasource_id={virtual_dataset.id}&datasource_type=table" | |||
) | |||
rv = test_client.post(uri, json=None) | |||
assert rv.status_code == 200 | |||
assert rv.status_code == 400 |
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.
why did this changed to a 400 now?
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.
Werkzeug 2.1.0 changelog:
Request.get_json() will raise a 400 BadRequest error if the Content-Type header is not application/json. This makes a very common source of confusion more visible. pallets/werkzeug#2339
If there's no json param or json=None
then 400 http status will be thrown
Great work! @EugeneTorap |
@EugeneTorap @dpgaspar Seeing that the compiled |
@cwegener @EugeneTorap the lock files should preferably be updated via |
I find that Anyway. Sounds like there is a need for either discussion of the Python dependency management options, or a better checking of consistent |
We should probably add a check to to our linting workflow ensure the compiled lock files are correct. Should be easy. |
SUMMARY
The PR bump the next flask libs:
ADDITIONAL INFORMATION