Manually merge PR131 to add a configurable limit on the size of cat.py output data served in relation to memory exhaustion as explained in github issue 119.

jonasbardino · jonasbardino · commit 8fcc3c30d148 · 2024-10-11T07:50:33.000Z
git-svn-id: svn+ssh://svn.code.sf.net/p/migrid/code/trunk@6150 b75ad72c-e7d7-11dd-a971-7dbc132099af
diff --git a/mig/shared/configuration.py b/mig/shared/configuration.py
@@ -181,6 +181,7 @@ def fix_missing(config_file, verbose=True):
         'mig_system_storage': '~/state/mig_system_storage',
         'mig_system_run': '~/state/mig_system_run/',
         'wwwpublic': '~/state/wwwpublic/',
+        'wwwserve_max_bytes': -1,
         'vm_home': '~/state/vm_home',
         'server_cert': '~/certs/cert.pem',
         'server_key': '~/certs/key.pem',
@@ -600,6 +601,7 @@ def fix_missing(config_file, verbose=True):
     'myfiles_py_location': '',
     'public_key_file': '',
     'wwwpublic': '',
+    'wwwserve_max_bytes': -1,
     # Virtual machine VNC proxy helpers
     'vm_home': '',
     'vm_proxy_host': '',
diff --git a/mig/shared/functionality/cat.py b/mig/shared/functionality/cat.py
@@ -29,6 +29,7 @@
 
 from __future__ import absolute_import
 
+import errno
 import glob
 import mimetypes
 import os
@@ -46,6 +47,36 @@
 from mig.shared.validstring import valid_user_path
 
 
+def _check_serve_permitted(configuration, paths):
+    """Given a set of paths ensure that their total size is within limits."""
+
+    wwwserve_max_bytes = configuration.wwwserve_max_bytes
+
+    try:
+        serve_paths_stat_results = (os.stat(path) for path in paths)
+    except OSError as oserr:
+        # Backwards compatible version of the FileNotFoundError from py3
+        if oserr.errno == errno.ENOENT:
+            return False
+
+    serve_paths_total_bytes = sum(
+        st.st_size for st in serve_paths_stat_results)
+
+    if wwwserve_max_bytes > -1 and serve_paths_total_bytes > wwwserve_max_bytes:
+        return False
+
+    return True
+
+
+def _render_error_text_for_serve_limit(configuration):
+    """Helper to inform about suitable alternatives when above limit"""
+    limit = configuration.wwwserve_max_bytes
+    alternatives = ', '.join(configuration.storage_protocols)
+    return ("Site configuration prevents web serving contents bigger than "
+            "%d bytes - please use better alternatives (%s) to retrieve large "
+            "data.") % (limit, alternatives)
+
+
 def signature():
     """Signature of the main function"""
 
@@ -166,6 +197,12 @@ def _main(configuration, logger, op_name='', output_objects=[], client_id=None,
                                    'name': pattern})
             status = returnvalues.FILE_NOT_FOUND
 
+        if not _check_serve_permitted(configuration, paths=match):
+            status = returnvalues.REJECTED_ERROR
+            text = _render_error_text_for_serve_limit(configuration)
+            output_objects.append({'object_type': 'error_text', 'text': text})
+            return (output_objects, status)
+
         for abs_path in match:
             output_lines = []
             relative_path = abs_path.replace(base_dir, '')
diff --git a/mig/shared/returnvalues.py b/mig/shared/returnvalues.py
@@ -46,3 +46,7 @@
 USER_NOT_CREATED = (201, 'USER_NOT_CREATED')
 OUTPUT_VALIDATION_ERROR = (202, 'The output the server '
                             + 'has generated could not be validated')
+
+# REQUEST ERRORS
+
+REJECTED_ERROR = (422, 'REJECTED')
diff --git a/tests/data/loading.gif b/tests/data/loading.gif
@@ -0,0 +1 @@
+../../mig/images/loading.gif
diff --git a/tests/fixture/mig_shared_configuration--new.json b/tests/fixture/mig_shared_configuration--new.json
@@ -665,5 +665,6 @@
   "workflows_vgrid_patterns_home": "",
   "workflows_vgrid_recipes_home": "",
   "workflows_vgrid_tasks_home": "",
-  "wwwpublic": ""
+  "wwwpublic": "",
+  "wwwserve_max_bytes": -1
 }
diff --git a/tests/support/__init__.py b/tests/support/__init__.py
@@ -42,7 +42,7 @@
 
 from tests.support.configsupp import FakeConfiguration
 from tests.support.suppconst import MIG_BASE, TEST_BASE, TEST_FIXTURE_DIR, \
-    TEST_OUTPUT_DIR
+    TEST_OUTPUT_DIR, TEST_DATA_DIR
 
 PY2 = (sys.version_info[0] == 2)
 
diff --git a/tests/support/suppconst.py b/tests/support/suppconst.py
@@ -31,6 +31,7 @@
 # Use abspath for __file__ on Py2
 _SUPPORT_DIR = os.path.dirname(os.path.abspath(__file__))
 TEST_BASE = os.path.normpath(os.path.join(_SUPPORT_DIR, ".."))
+TEST_DATA_DIR = os.path.join(TEST_BASE, "data")
 TEST_FIXTURE_DIR = os.path.join(TEST_BASE, "fixture")
 TEST_OUTPUT_DIR = os.path.join(TEST_BASE, "output")
 MIG_BASE = os.path.realpath(os.path.join(TEST_BASE, ".."))
diff --git a/tests/test_mig_shared_functionality_cat.py b/tests/test_mig_shared_functionality_cat.py
@@ -25,16 +25,16 @@
 # --- END_HEADER ---
 #
 
-"""Unit tests of the MiG functionality file implementing the cat backed"""
+"""Unit tests of the MiG functionality file implementing the cat backend"""
 
 from __future__ import print_function
 import importlib
 import os
 import shutil
 import sys
 
-from tests.support import MIG_BASE, MigTestCase, testmain, fixturefile, \
-    fixturefile_normname, ensure_dirs_exist, temppath
+from tests.support import MIG_BASE, TEST_DATA_DIR, MigTestCase, testmain, \
+    fixturefile, fixturefile_normname, ensure_dirs_exist, temppath
 
 from mig.shared.base import client_id_dir
 from mig.shared.functionality.cat import _main as main
@@ -85,19 +85,74 @@ def before_each(self):
         temppath(self.test_user_dir, self, skip_output_anchor=True)
         self.test_environ = create_http_environ(self.configuration)
 
-    def test_returns_file_output_with_single_file_match(self):
+    def test_file_serving_a_single_file_match(self):
         with open(os.path.join(self.test_user_dir, 'foobar.txt'), 'w'):
             pass
         payload = {
             'path': ['foobar.txt'],
         }
 
         (output_objects, status) = main(self.configuration, self.logger,
-                                        client_id=self.TEST_CLIENT_ID, user_arguments_dict=payload, environ=self.test_environ)
+                                        client_id=self.TEST_CLIENT_ID,
+                                        user_arguments_dict=payload,
+                                        environ=self.test_environ)
         self.assertEqual(len(output_objects), 1)
         output_obj = output_objects[0]
         self.assertEqual(output_obj['object_type'], 'file_output')
 
+    def test_file_serving_at_limit(self):
+        test_binary_file = os.path.realpath(
+            os.path.join(TEST_DATA_DIR, 'loading.gif'))
+        test_binary_file_size = os.stat(test_binary_file).st_size
+        with open(test_binary_file, 'rb') as fh_test_file:
+            test_binary_file_data = fh_test_file.read()
+        shutil.copyfile(test_binary_file, os.path.join(
+            self.test_user_dir, 'loading.gif'))
+        payload = {
+            'output_format': ['file'],
+            'path': ['loading.gif'],
+        }
+
+        self.configuration.wwwserve_max_bytes = test_binary_file_size
+
+        (output_objects, status) = main(self.configuration, self.logger,
+                                        client_id=self.TEST_CLIENT_ID,
+                                        user_arguments_dict=payload,
+                                        environ=self.test_environ)
+        # TODO: two file_output objects seem to be returned
+        self.assertEqual(len(output_objects), 3)
+        relevant_obj = output_objects[2]
+        self.assertEqual(relevant_obj['object_type'], 'file_output')
+        self.assertEqual(len(relevant_obj['lines']), 1)
+        self.assertEqual(relevant_obj['lines'][0], test_binary_file_data)
+
+    def test_file_serving_over_limit(self):
+        test_binary_file = os.path.realpath(os.path.join(TEST_DATA_DIR,
+                                                         'loading.gif'))
+        test_binary_file_size = os.stat(test_binary_file).st_size
+        with open(test_binary_file, 'rb') as fh_test_file:
+            test_binary_file_data = fh_test_file.read()
+        shutil.copyfile(test_binary_file, os.path.join(self.test_user_dir,
+                                                       'loading.gif'))
+        payload = {
+            'output_format': ['file'],
+            'path': ['loading.gif'],
+        }
+
+        self.configuration.wwwserve_max_bytes = test_binary_file_size - 1
+
+        (output_objects, status) = main(self.configuration, self.logger,
+                                        client_id=self.TEST_CLIENT_ID,
+                                        user_arguments_dict=payload,
+                                        environ=self.test_environ)
+        self.assertEqual(len(output_objects), 4)
+        relevant_obj = output_objects[3]
+        self.assertEqual(relevant_obj['object_type'], 'error_text')
+        self.assertEqual(relevant_obj['text'],
+                         "Site configuration prevents web serving contents "
+                         "bigger than 3896 bytes - please use better "
+                         "alternatives (sftp) to retrieve large data.")
+
 
 if __name__ == '__main__':
     testmain()

Original file line number	Diff line number	Diff line change
`@@ -665,5 +665,6 @@`
`665`	`665`	`"workflows_vgrid_patterns_home": "",`
`666`	`666`	`"workflows_vgrid_recipes_home": "",`
`667`	`667`	`"workflows_vgrid_tasks_home": "",`
`668`		`- "wwwpublic": ""`
	`668`	`+ "wwwpublic": "",`
	`669`	`+ "wwwserve_max_bytes": -1`
`669`	`670`	`}`