alexkruc commented on code in PR #26886:
URL: https://github.com/apache/airflow/pull/26886#discussion_r988853924
##########
airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -902,14 +911,21 @@ def download_file(self, key: str, bucket_name: str | None
= None, local_path: st
else:
raise e
- with NamedTemporaryFile(dir=local_path, prefix='airflow_tmp_',
delete=False) as local_tmp_file:
+ if preserve_file_name:
+ local_dir = local_path if local_path else gettempdir()
Review Comment:
WDYM?
I've tested it locally in Airflow docker and in Breeze, and the
`gettempdir()` method retunes `/tmp` on all Linux envs..
When the `dir` parameter is not provided to the `NamedTemporaryFile`, it
also called directly:
https://github.com/python/cpython/blob/0d68879104dfb392d31e52e25dcb0661801a0249/Lib/tempfile.py#L126
I do not quite understand why it may cause a vulnerability.
Do you think it's better to stay with the older implementation of renaming
the file after it's already been created? I think that this way is a bit
cleaner, but I'm also ok with also "reverting" to the old flow..
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
