You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "yo1956 (via GitHub)" <gi...@apache.org> on 2023/12/24 05:58:50 UTC
[PR] Add flask config: MAX_CONTENT_LENGTH (#35196) [airflow]
yo1956 opened a new pull request, #36401:
URL: https://github.com/apache/airflow/pull/36401
Add flask config to limit the allowed payload.
https://github.com/apache/airflow/issues/35196
I set it to limit to 1MB.
Before and after adding this config, I tried sending a string larger than 1MB from the Edit User page.
Before adding the config, it was possible to send; after adding, I confirmed that the attached error page was received.
<img width="516" alt="request_entity_too_large_2023-12-24 14 35 41" src="https://github.com/apache/airflow/assets/39612448/c32617a0-c835-4a91-bb41-3ea853917e3e">
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add flask config: `MAX_CONTENT_LENGTH` [airflow]
Posted by "hussein-awala (via GitHub)" <gi...@apache.org>.
hussein-awala commented on code in PR #36401:
URL: https://github.com/apache/airflow/pull/36401#discussion_r1435891375
##########
airflow/www/app.py:
##########
@@ -75,6 +75,9 @@ def create_app(config=None, testing=False):
flask_app.config["PERMANENT_SESSION_LIFETIME"] = timedelta(minutes=settings.get_session_lifetime_config())
+ limit_in_mb = 1
Review Comment:
IMHO, this value should be configurable by Airflow webserver configurations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add flask config: `MAX_CONTENT_LENGTH` [airflow]
Posted by "dirrao (via GitHub)" <gi...@apache.org>.
dirrao commented on PR #36401:
URL: https://github.com/apache/airflow/pull/36401#issuecomment-1868548193
> Thank you for your comment.
>
> > How about triggering a DAG with input configuration?
>
> Sorry, what does this mean?
>
> > Does it require more than 1MB?
>
> I'm not really sure what would be an appropriate value. Do you have any ideas?
>
We can pass the JSON payload while triggering the DAG from the Airflow UI. JSON payload can be arbitrary length. It is much bigger. I haven't come across a use case where we pass more than 1MB. However, I am just checking.
> > If we move this configuration to the web server section in airflow cfg, then we can set based on our requirements.
>
> Does this mean instead of hardcoding the value, we can set an item in airflow.cfg and get that value as a flask config?
>
Trying to see if can avoid hardcoding in the code.
> > For user profile information, is it feasible to restrict the size of form fields rather than relying on the defaults?
>
> Certainly, I feel it's feasible. Sorry, could you please clarify what you mean by "rather than relying on the defaults"?
I mean relying on the same default value for multiple forms in the Airflow UI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add flask config: `MAX_CONTENT_LENGTH` [airflow]
Posted by "yo1956 (via GitHub)" <gi...@apache.org>.
yo1956 commented on code in PR #36401:
URL: https://github.com/apache/airflow/pull/36401#discussion_r1435892468
##########
airflow/www/app.py:
##########
@@ -75,6 +75,9 @@ def create_app(config=None, testing=False):
flask_app.config["PERMANENT_SESSION_LIFETIME"] = timedelta(minutes=settings.get_session_lifetime_config())
+ limit_in_mb = 1
Review Comment:
@hussein-awala
As @dirrao also mentioned, indeed that makes sense. I'll start working on this first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add flask config: `MAX_CONTENT_LENGTH` [airflow]
Posted by "yo1956 (via GitHub)" <gi...@apache.org>.
yo1956 commented on PR #36401:
URL: https://github.com/apache/airflow/pull/36401#issuecomment-1868539949
Thank you for your comment.
> How about triggering a DAG with input configuration?
Sorry, what does this mean?
> Does it require more than 1MB?
I'm not really sure what would be an appropriate value. Do you have any ideas?
> If we move this configuration to the web server section in airflow cfg, then we can set based on our requirements.
Does this mean instead of hardcoding the value, we can set an item in airflow.cfg and get that value as a flask config?
> For user profile information, is it feasible to restrict the size of form fields rather than relying on the defaults?
Certainly, I feel it's feasible. Sorry, could you please clarify what you mean by "rather than relying on the defaults"?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add flask config: `MAX_CONTENT_LENGTH` [airflow]
Posted by "yo1956 (via GitHub)" <gi...@apache.org>.
yo1956 commented on code in PR #36401:
URL: https://github.com/apache/airflow/pull/36401#discussion_r1436464445
##########
airflow/www/app.py:
##########
@@ -75,6 +75,9 @@ def create_app(config=None, testing=False):
flask_app.config["PERMANENT_SESSION_LIFETIME"] = timedelta(minutes=settings.get_session_lifetime_config())
+ limit_in_mb = 1
Review Comment:
@hussein-awala @dirrao
I made the value configurable by placing it in the webserver section of airflow.cfg.
Please check it when you have time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add flask config: `MAX_CONTENT_LENGTH` [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #36401:
URL: https://github.com/apache/airflow/pull/36401#discussion_r1438580095
##########
airflow/config_templates/config.yml:
##########
@@ -1855,6 +1855,13 @@ webserver:
type: boolean
example: "False"
default: "False"
+ allowed_payload:
+ description: |
+ The maximum size of the request payload (in MB) that can be sent.
+ version_added: 2.9.0
Review Comment:
Do we want it in 2.9.0 or 2.8.1?
I think we can consider this one also as bug fix?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add flask config: `MAX_CONTENT_LENGTH` [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #36401:
URL: https://github.com/apache/airflow/pull/36401#discussion_r1438589629
##########
airflow/config_templates/config.yml:
##########
@@ -1855,6 +1855,13 @@ webserver:
type: boolean
example: "False"
default: "False"
+ allowed_payload_size:
+ description: |
+ The maximum size of the request payload (in MB) that can be sent.
+ version_added: 2.9.0
Review Comment:
```suggestion
version_added: 2.8.1
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add flask config: `MAX_CONTENT_LENGTH` [airflow]
Posted by "hussein-awala (via GitHub)" <gi...@apache.org>.
hussein-awala merged PR #36401:
URL: https://github.com/apache/airflow/pull/36401
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add flask config: `MAX_CONTENT_LENGTH` [airflow]
Posted by "hussein-awala (via GitHub)" <gi...@apache.org>.
hussein-awala commented on code in PR #36401:
URL: https://github.com/apache/airflow/pull/36401#discussion_r1438582934
##########
airflow/www/app.py:
##########
@@ -75,6 +75,8 @@ def create_app(config=None, testing=False):
flask_app.config["PERMANENT_SESSION_LIFETIME"] = timedelta(minutes=settings.get_session_lifetime_config())
+ flask_app.config["MAX_CONTENT_LENGTH"] = conf.getfloat("webserver", "allowed_payload") * 1024 * 1024
Review Comment:
```suggestion
flask_app.config["MAX_CONTENT_LENGTH"] = conf.getfloat("webserver", "allowed_payload_size") * 1024 * 1024
```
##########
airflow/config_templates/config.yml:
##########
@@ -1855,6 +1855,13 @@ webserver:
type: boolean
example: "False"
default: "False"
+ allowed_payload:
Review Comment:
```suggestion
allowed_payload_size:
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add flask config: `MAX_CONTENT_LENGTH` [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #36401:
URL: https://github.com/apache/airflow/pull/36401#issuecomment-1872511542
@hussein-awala ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add flask config: `MAX_CONTENT_LENGTH` [airflow]
Posted by "yo1956 (via GitHub)" <gi...@apache.org>.
yo1956 commented on code in PR #36401:
URL: https://github.com/apache/airflow/pull/36401#discussion_r1438594348
##########
airflow/www/app.py:
##########
@@ -75,6 +75,8 @@ def create_app(config=None, testing=False):
flask_app.config["PERMANENT_SESSION_LIFETIME"] = timedelta(minutes=settings.get_session_lifetime_config())
+ flask_app.config["MAX_CONTENT_LENGTH"] = conf.getfloat("webserver", "allowed_payload") * 1024 * 1024
Review Comment:
committed in 3da440410c5b82f12e56cafa87af9a8647d73f88
##########
airflow/config_templates/config.yml:
##########
@@ -1855,6 +1855,13 @@ webserver:
type: boolean
example: "False"
default: "False"
+ allowed_payload:
Review Comment:
committed in 3da440410c5b82f12e56cafa87af9a8647d73f88
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add flask config: `MAX_CONTENT_LENGTH` [airflow]
Posted by "yo1956 (via GitHub)" <gi...@apache.org>.
yo1956 commented on code in PR #36401:
URL: https://github.com/apache/airflow/pull/36401#discussion_r1438594348
##########
airflow/www/app.py:
##########
@@ -75,6 +75,8 @@ def create_app(config=None, testing=False):
flask_app.config["PERMANENT_SESSION_LIFETIME"] = timedelta(minutes=settings.get_session_lifetime_config())
+ flask_app.config["MAX_CONTENT_LENGTH"] = conf.getfloat("webserver", "allowed_payload") * 1024 * 1024
Review Comment:
commited in 3da440410c5b82f12e56cafa87af9a8647d73f88
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add flask config: `MAX_CONTENT_LENGTH` [airflow]
Posted by "yo1956 (via GitHub)" <gi...@apache.org>.
yo1956 commented on code in PR #36401:
URL: https://github.com/apache/airflow/pull/36401#discussion_r1438595200
##########
airflow/config_templates/config.yml:
##########
@@ -1855,6 +1855,13 @@ webserver:
type: boolean
example: "False"
default: "False"
+ allowed_payload:
+ description: |
+ The maximum size of the request payload (in MB) that can be sent.
+ version_added: 2.9.0
Review Comment:
committed in a1e667e33f8456e3c62c5c5ba7331f4f3e413e0e
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add flask config: `MAX_CONTENT_LENGTH` [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #36401:
URL: https://github.com/apache/airflow/pull/36401#discussion_r1438580496
##########
airflow/config_templates/config.yml:
##########
@@ -1855,6 +1855,13 @@ webserver:
type: boolean
example: "False"
default: "False"
+ allowed_payload:
+ description: |
+ The maximum size of the request payload (in MB) that can be sent.
+ version_added: 2.9.0
Review Comment:
Yes. That works for me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org