Commit34a0362

Jesse Whitehouse

committed

Resolve relative paths before comparing row.localFile to uploads_base_path

Applies feedback from PR reviewSigned-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>

1 parentcdbe2d6 commit34a0362Copy full SHA for 34a0362

File tree

+26

-3

lines changed

+26

-3

lines changed

Lines changed: 8 additions & 3 deletions

Original file line number	Diff line number	Diff line change
`@@ -312,18 +312,23 @@ def _handle_staging_operation(self, uploads_base_path: str):`
`312`	`312`	`raiseError(`
`313`	`313`	`"You must provide an uploads_base_path when initialising a connection to perform ingestion commands"`
`314`	`314`	`)`
`315`		`-`
	`315`	`+`
`316`	`316`	`row=self.active_result_set.fetchone()`
`317`	`317`
	`318`	`+# Must set to None in cases where server response does not include localFile`
	`319`	`+abs_localFile=None`
	`320`	`+`
`318`	`321`	`ifgetattr(row,"localFile",None):`
`319`		`-ifos.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`	`+ifos.path.commonpath([abs_localFile,abs_uploads_base_path])!=abs_uploads_base_path:`
`320`	`325`	`raiseError("Local file operations are restricted to paths within the configured uploads_base_path")`
`321`	`326`
`322`	`327`	`# TODO: Experiment with DBR sending real headers.`
`323`	`328`	`# The specification says headers will be in JSON format but the current null value is actually an empty list []`
`324`	`329`	`handler_args= {`
`325`	`330`	`"presigned_url":row.presignedUrl,`
`326`		`-"local_file":getattr(row,"localFile",None),`
	`331`	`+"local_file":abs_localFile,`
`327`	`332`	`"headers":json.loads(row.headersor"{}"),`
`328`	`333`	`}`
`329`	`334`

Lines changed: 18 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -741,6 +741,24 @@ def test_staging_ingestion_put_fails_if_localFile_not_in_uploads_base_path(self)`
`741`	`741`	`query=f"PUT '{temp_path}' INTO 'stage://tmp/{self.staging_ingestion_user}/tmp/11/15/file1.csv' OVERWRITE"`
`742`	`742`	`cursor.execute(query)`
`743`	`743`
	`744`	`+deftest_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`	`+withpytest.raises(Error):`
	`756`	`+withself.connection(extra_params={"uploads_base_path":uploads_base_path})asconn:`
	`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`	`+`
`744`	`762`
`745`	`763`	`defmain(cli_args):`
`746`	`764`	`globalget_args_from_env`

Comments

(0)