From 3061d943b4bc1aba36239fe25292de67e869d381 Mon Sep 17 00:00:00 2001 From: Stefan Janssen Date: Wed, 17 Sep 2025 09:09:36 +0200 Subject: [PATCH 1/4] discriminate nginx and direct requests and either use nginx or tornado file delivery method respectively --- .../cloud_handlers/file_transfer_handlers.py | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py b/qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py index 2287c5c31..19a3f78e6 100644 --- a/qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py +++ b/qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py @@ -37,20 +37,33 @@ def get(self, requested_filepath): raise HTTPError(403, reason=( "The requested file is not present in Qiita's BASE_DATA_DIR!")) - # delivery of the file via nginx requires replacing the basedatadir - # with the prefix defined in the nginx configuration for the - # base_data_dir, '/protected/' by default - protected_filepath = filepath.replace(basedatadir, '/protected') - self.set_header('Content-Type', 'application/octet-stream') self.set_header('Content-Transfer-Encoding', 'binary') - self.set_header('X-Accel-Redirect', protected_filepath) self.set_header('Content-Description', 'File Transfer') self.set_header('Expires', '0') self.set_header('Cache-Control', 'no-cache') - self.set_header('Content-Disposition', - 'attachment; filename=%s' % os.path.basename( - protected_filepath)) + + # We here need to differentiate a request that comes directly to the + # qiita instance (happens in testing) or was redirected through nginx + # (should be the default). If nginx, we can use nginx' fast file + # delivery mechanisms, otherwise, we need to send via slower tornado. + # We indirectly infer this by looking for the "X-Forwarded-For" header, + # which should only exists when redirectred through nginx. + if self.request.headers.get('X-Forwarded-For') is None: + self.set_header('Content-Disposition', + 'attachment; filename=%s' % os.path.basename(filepath)) + with open(filepath, "rb") as f: + self.write(f.read()) + else: + # delivery of the file via nginx requires replacing the basedatadir + # with the prefix defined in the nginx configuration for the + # base_data_dir, '/protected/' by default + protected_filepath = filepath.replace(basedatadir, '/protected') + self.set_header('X-Accel-Redirect', protected_filepath) + self.set_header('Content-Disposition', + 'attachment; filename=%s' % os.path.basename( + protected_filepath)) + self.finish() From cfe606af28c9561034d095edb28a0829b1487031 Mon Sep 17 00:00:00 2001 From: Stefan Janssen Date: Wed, 17 Sep 2025 09:11:37 +0200 Subject: [PATCH 2/4] codestyle --- qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py b/qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py index 19a3f78e6..39b435d09 100644 --- a/qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py +++ b/qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py @@ -50,7 +50,8 @@ def get(self, requested_filepath): # We indirectly infer this by looking for the "X-Forwarded-For" header, # which should only exists when redirectred through nginx. if self.request.headers.get('X-Forwarded-For') is None: - self.set_header('Content-Disposition', + self.set_header( + 'Content-Disposition', 'attachment; filename=%s' % os.path.basename(filepath)) with open(filepath, "rb") as f: self.write(f.read()) @@ -60,7 +61,8 @@ def get(self, requested_filepath): # base_data_dir, '/protected/' by default protected_filepath = filepath.replace(basedatadir, '/protected') self.set_header('X-Accel-Redirect', protected_filepath) - self.set_header('Content-Disposition', + self.set_header( + 'Content-Disposition', 'attachment; filename=%s' % os.path.basename( protected_filepath)) From 71526bbca08d83fcee4d2d969b93c1830777cf21 Mon Sep 17 00:00:00 2001 From: Stefan Janssen Date: Thu, 18 Sep 2025 11:23:51 +0200 Subject: [PATCH 3/4] don't complain about overwriting files IF in test mode --- qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py | 4 +++- .../cloud_handlers/tests/test_file_transfer_handlers.py | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py b/qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py index 39b435d09..6ca91bc35 100644 --- a/qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py +++ b/qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py @@ -92,7 +92,9 @@ def post(self): filepath = filepath[len(os.sep):] filepath = os.path.abspath(os.path.join(basedatadir, filepath)) - if os.path.exists(filepath): + # prevent overwriting existing files, except in test mode + if os.path.exists(filepath) and \ + (not qiita_config.test_environment): raise HTTPError(403, reason=( "The requested file is already " "present in Qiita's BASE_DATA_DIR!")) diff --git a/qiita_pet/handlers/cloud_handlers/tests/test_file_transfer_handlers.py b/qiita_pet/handlers/cloud_handlers/tests/test_file_transfer_handlers.py index 12dfac9b9..8967df095 100644 --- a/qiita_pet/handlers/cloud_handlers/tests/test_file_transfer_handlers.py +++ b/qiita_pet/handlers/cloud_handlers/tests/test_file_transfer_handlers.py @@ -5,6 +5,7 @@ from qiita_db.handlers.tests.oauthbase import OauthTestingBase import qiita_db as qdb +from qiita_core.qiita_settings import qiita_config class FetchFileFromCentralHandlerTests(OauthTestingBase): @@ -70,7 +71,11 @@ def test_post(self): # check if error is raised, if file already exists with open(fp_source, 'rb') as fh: + # we need to let qiita thinks for this test, to NOT be in test mode + oldval = qiita_config.test_environment + qiita_config.test_environment = False obs = self.post_authed(endpoint, files={'bar/': fh}) + qiita_config.test_environment = oldval self.assertIn("already present in Qiita's BASE_DATA_DIR!", obs.reason) From c7924ca5b19874817cc244ef986295e4ad38ac9f Mon Sep 17 00:00:00 2001 From: Stefan Janssen Date: Thu, 18 Sep 2025 12:21:32 +0200 Subject: [PATCH 4/4] don't read config from file, but from DB --- .../cloud_handlers/file_transfer_handlers.py | 5 ++--- .../tests/test_file_transfer_handlers.py | 12 ++++++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py b/qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py index 6ca91bc35..321ee30f4 100644 --- a/qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py +++ b/qiita_pet/handlers/cloud_handlers/file_transfer_handlers.py @@ -3,7 +3,7 @@ from tornado.web import HTTPError, RequestHandler from tornado.gen import coroutine -from qiita_core.util import execute_as_transaction +from qiita_core.util import execute_as_transaction, is_test_environment from qiita_db.handlers.oauth2 import authenticate_oauth from qiita_core.qiita_settings import qiita_config @@ -93,8 +93,7 @@ def post(self): filepath = os.path.abspath(os.path.join(basedatadir, filepath)) # prevent overwriting existing files, except in test mode - if os.path.exists(filepath) and \ - (not qiita_config.test_environment): + if os.path.exists(filepath) and (not is_test_environment()): raise HTTPError(403, reason=( "The requested file is already " "present in Qiita's BASE_DATA_DIR!")) diff --git a/qiita_pet/handlers/cloud_handlers/tests/test_file_transfer_handlers.py b/qiita_pet/handlers/cloud_handlers/tests/test_file_transfer_handlers.py index 8967df095..9a7cec722 100644 --- a/qiita_pet/handlers/cloud_handlers/tests/test_file_transfer_handlers.py +++ b/qiita_pet/handlers/cloud_handlers/tests/test_file_transfer_handlers.py @@ -5,7 +5,7 @@ from qiita_db.handlers.tests.oauthbase import OauthTestingBase import qiita_db as qdb -from qiita_core.qiita_settings import qiita_config +from qiita_db.sql_connection import TRN class FetchFileFromCentralHandlerTests(OauthTestingBase): @@ -72,10 +72,14 @@ def test_post(self): # check if error is raised, if file already exists with open(fp_source, 'rb') as fh: # we need to let qiita thinks for this test, to NOT be in test mode - oldval = qiita_config.test_environment - qiita_config.test_environment = False + with TRN: + TRN.add("UPDATE settings SET test = False") + TRN.execute() obs = self.post_authed(endpoint, files={'bar/': fh}) - qiita_config.test_environment = oldval + # reset test mode to true + with TRN: + TRN.add("UPDATE settings SET test = True") + TRN.execute() self.assertIn("already present in Qiita's BASE_DATA_DIR!", obs.reason)