steveahnahn commented on code in PR #69850:
URL: https://github.com/apache/airflow/pull/69850#discussion_r3693165926
##########
task-sdk/src/airflow/sdk/io/path.py:
##########
@@ -339,6 +340,25 @@ def size(self) -> int:
"""Size in bytes of the file at this path."""
return self.fs.size(self.path)
+ def _raise_if_remote_keys_escape(self, local_dir: str, **kwargs) -> None:
+ """
+ Refuse a recursive download when a remote object key resolves outside
``local_dir``.
+
+ Object-store keys are arbitrary strings and may contain ``..``
segments written by
+ anyone who can put objects in the source prefix; ``fs.get`` follows
them verbatim and
+ would write outside the destination directory.
+ """
+ dst_root = os.path.realpath(local_dir)
+ for src_key in self.fs.expand_path(self.path, recursive=True,
**kwargs):
+ if self.fs.isdir(src_key):
+ continue
Review Comment:
This adds one isdir round trip per object, on top of a second full
expand_path traversal. Measured against main with a fake remote fs: 10 objects
goes from 17 isdir calls to 34, 50 from 57 to 114, 200 from 207 to 414. It
exactly doubles them, so the cost grows with object count on a path that is
already doing a bulk transfer.
self.fs.find(self.path, **kwargs) returns files only and gives an identical
file set to the expand_path plus per-key isdir filter, so swapping it in would
drop the per-key calls entirely and make the guard close to free.
--
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]
