Skip to content

Commit 34a0362

Browse files
author
Jesse Whitehouse
committed
Resolve relative paths before comparing row.localFile to uploads_base_path
Applies feedback from PR review Signed-off-by: Jesse Whitehouse <[email protected]>
1 parent cdbe2d6 commit 34a0362

File tree

2 files changed

+26
-3
lines changed

2 files changed

+26
-3
lines changed

src/databricks/sql/client.py

+8-3
Original file line numberDiff line numberDiff line change
@@ -312,18 +312,23 @@ def _handle_staging_operation(self, uploads_base_path: str):
312312
raise Error(
313313
"You must provide an uploads_base_path when initialising a connection to perform ingestion commands"
314314
)
315-
315+
316316
row = self.active_result_set.fetchone()
317317

318+
# Must set to None in cases where server response does not include localFile
319+
abs_localFile = None
320+
318321
if getattr(row, "localFile", None):
319-
if os.path.commonpath([row.localFile, uploads_base_path]) != uploads_base_path:
322+
abs_localFile = os.path.abspath(row.localFile)
323+
abs_uploads_base_path = os.path.abspath(uploads_base_path)
324+
if os.path.commonpath([abs_localFile, abs_uploads_base_path]) != abs_uploads_base_path:
320325
raise Error("Local file operations are restricted to paths within the configured uploads_base_path")
321326

322327
# TODO: Experiment with DBR sending real headers.
323328
# The specification says headers will be in JSON format but the current null value is actually an empty list []
324329
handler_args = {
325330
"presigned_url": row.presignedUrl,
326-
"local_file": getattr(row, "localFile", None),
331+
"local_file": abs_localFile,
327332
"headers": json.loads(row.headers or "{}"),
328333
}
329334

tests/e2e/driver_tests.py

+18
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,24 @@ def test_staging_ingestion_put_fails_if_localFile_not_in_uploads_base_path(self)
741741
query = f"PUT '{temp_path}' INTO 'stage://tmp/{self.staging_ingestion_user}/tmp/11/15/file1.csv' OVERWRITE"
742742
cursor.execute(query)
743743

744+
def test_staging_ingestion_put_fails_if_absolute_localFile_not_in_uploads_base_path(self):
745+
"""
746+
This test confirms that uploads_base_path and target_file are resolved into absolute paths.
747+
"""
748+
749+
# If these two paths are not resolved absolutely, they appear to share a common path of /var/www/html
750+
# after resolution their common path is only /var/www which should raise an exception
751+
# Because the common path must always be equal to uploads_base_path
752+
uploads_base_path = "/var/www/html"
753+
target_file = "/var/www/html/../html1/not_allowed.html"
754+
755+
with pytest.raises(Error):
756+
with self.connection(extra_params={"uploads_base_path": uploads_base_path}) as conn:
757+
cursor = conn.cursor()
758+
query = f"PUT '{target_file}' INTO 'stage://tmp/{self.staging_ingestion_user}/tmp/11/15/file1.csv' OVERWRITE"
759+
cursor.execute(query)
760+
761+
744762

745763
def main(cli_args):
746764
global get_args_from_env

0 commit comments

Comments
 (0)