fix(ingest/s3): Path spec aware folder traversal #8095
Conversation
…s not a partition column
github-actions
bot
added
the
ingestion
|
|
||
| for i in range(slash_to_remove_from_glob): | ||
| glob_include = glob_include.rsplit("/", 1)[0] | ||
|
|
There was a problem hiding this comment.
Why are we removing one additional slash than slash_to_remove_from_glob ?
There was a problem hiding this comment.
I added a comment and restructured the code to make it more clear.
| bucket_name=bucket_name, folder=sorted_dirs[0] + "/" | ||
| ) | ||
| for dir in sorted_dirs: | ||
| if path_spec.dir_allowed(f"{protocol}" + bucket_name + "/" + dir + "/"): |
There was a problem hiding this comment.
| if path_spec.dir_allowed(f"{protocol}" + bucket_name + "/" + dir + "/"): | |
| if path_spec.dir_allowed(f"{protocol}{bucket_name}/{dir}/"): |
| dir_to_process = self.get_dir_to_process( | ||
| bucket_name=bucket_name, folder=f + "/" | ||
| bucket_name=bucket_name, | ||
| folder=f + "/", |
There was a problem hiding this comment.
Not related to this PR. but this suffix with "/" could be moved inside get_dir_to_process before the call to list_folders.
There was a problem hiding this comment.
Is this file added intentionally ?
There was a problem hiding this comment.
Yes, without the new logic, this file would have been picked up.
| return protocol | ||
| else: | ||
| raise ValueError( | ||
| f"Unable to get protocol or invalid protocol form path: {path}" |
| for i in range(slash_to_remove_from_glob): | ||
| glob_include = glob_include.rsplit("/", 1)[0] |
There was a problem hiding this comment.
Could we get this via glob_include.rsplit("/", i)[0], so I guess glob_include.rsplit("/", slash_to_remove_from_glob + 1)[0] in total? If that's the first time it's defined
There was a problem hiding this comment.
Thanks, I made it more clear in the code.
Folder traversal needs to be aware of the path_spec and not go into folders which should not match with the path_spec.
The logic heavily relies on that nature of path spec which mandates to specify each folder level.
This pr depends on #8089
Checklist