Fix for issue #510.

Address attempting to concatenate strings to a an
empty byte string in the file_format output func.

The added tests here make use of changes to
migwsgi that make it possible to exercise the
entire output path. Bring those changes over
because having to write them again has no benefit.

Author: albu-diku

mig/shared/output.py

@@ -38,6 +38,7 @@
     exit(1)
 
 from past.builtins import basestring
+import inspect
 import os
 import sys
 import time
@@ -64,6 +65,20 @@
                          'yaml', 'xmlrpc', 'resource', 'json', 'file']
 
 
+def kwargs_for_functionality(main, configuration=None, environ=None):
+    """
+    Determine which additional arguments are supported by the
+    selected functionality method and arrange to to pass them.
+    """
+
+    parameters = inspect.signature(main).parameters
+
+    kwargs = dict()
+    if 'environ' in parameters:
+        kwargs['environ'] = environ
+    return kwargs
+
+
 def reject_main(client_id, user_arguments_dict):
     """A simple main-function to use if functionality backend is disabled"""
     output_objs = [bailout_title(None, 'Access Error'),
@@ -2697,9 +2712,6 @@ def file_format(configuration, ret_val, ret_msg, out_obj):
 
     # TODO: use wsgi file_wrapper helper here if out_obj has wsgi entry?
 
-    # NOTE: we expect binary data here and must use it consistently
-    file_content = b''
-
     # NOTE: carefully handle errors and ONLY render them when proper care has
     #       been taken to deliver them as actual output, to avoid that they end
     #       up hidden inside downloaded files.
@@ -2719,6 +2731,12 @@ def file_format(configuration, ret_val, ret_msg, out_obj):
         render_text, render_errors = True, True
     # _logger.debug("render output in file_format: %s (%s %s)" %
     #              (out_obj, render_text, render_errors))
+
+    if render_text:
+        file_content = ''
+    else:
+        file_content = b''
+
     for entry in out_obj:
         if entry['object_type'] == 'file_output':
             for line in entry['lines']:

mig/wsgi-bin/migwsgi.py

@@ -44,7 +44,8 @@
 from mig.shared.defaults import download_block_size, default_fs_coding
 from mig.shared.conf import get_configuration_object
 from mig.shared.objecttypes import get_object_type_info
-from mig.shared.output import validate, format_output, dummy_main, reject_main
+from mig.shared.output import validate, format_output, \
+    kwargs_for_functionality, dummy_main, reject_main
 from mig.shared.safeinput import valid_backend_name, html_escape, InputException
 from mig.shared.scriptinput import fieldstorage_to_dict, FixedFieldStorage
 
@@ -124,8 +125,11 @@ def stub(configuration, client_id, import_path, backend, user_arguments_dict,
         return (output_objects, returnvalues.INVALID_ARGUMENT)
 
     try:
+        main_kwargs = kwargs_for_functionality(main,
+                                               environ=environ)
         (output_objects, (ret_code, ret_msg)) = main(client_id,
-                                                     user_arguments_dict)
+                                                     user_arguments_dict,
+                                                     **main_kwargs)
     except Exception as err:
         import traceback
         _logger.error("%s script crashed:\n%s" % (_addr,

tests/support/wsgisupp.py

@@ -29,14 +29,32 @@
 
 from collections import namedtuple
 import codecs
+import importlib
 from io import BytesIO
+import os
+import sys
 from urllib.parse import urlencode, urlparse
 
-from werkzeug.datastructures import MultiDict
+from tests.support.suppconst import MIG_BASE
 
+TEXTUAL_CONTENT_TYPES = set(('text/plain', 'text/html', 'application/json'))
+OBJECTS_TYPE = 'objects'
 
-# named type representing the tuple that is passed to WSGI handlers
-_PreparedWsgi = namedtuple('_PreparedWsgi', ['environ', 'start_response'])
+
+def _import_forcibly(module_name, relative_module_dir=None):
+    """Custom import function to allow an import of a file for testing
+    that resides within a non-module directory."""
+
+    module_path = os.path.join(MIG_BASE, 'mig')
+    if relative_module_dir is not None:
+        module_path = os.path.join(module_path, relative_module_dir)
+    sys.path.append(module_path)
+    mod = importlib.import_module(module_name)
+    sys.path.pop(-1)  # do not leave the forced module path
+    return mod
+
+
+migwsgi = _import_forcibly('migwsgi', relative_module_dir='wsgi-bin')
 
 
 class FakeWsgiStartResponse:
@@ -51,7 +69,29 @@ def __call__(self, status, headers, exc=None):
         self.calls.append((status, headers, exc))
 
 
-def create_wsgi_environ(configuration, wsgi_url, method='GET', query=None, headers=None, form=None):
+def _urlencode_form(form_content):
+    """
+    Convert a data structure describing form contents to byte string
+    that can be directly sent as the body of an HTTP request.
+    """
+
+    field_key_and_value_pairs = []
+    if isinstance(form_content, dict):
+        for key, value in form_content.items():
+            if isinstance(value, list):
+                for item in value:
+                    field_key_and_value_pairs.append((key, item))
+                continue
+            field_key_and_value_pairs.append((key, value))
+    elif isinstance(form_content, list):
+        field_key_and_value_pairs = form_content
+    else:
+        raise AssertionError("invalid form content")
+    return urlencode(field_key_and_value_pairs, doseq=True).encode('ascii')
+
+
+def create_wsgi_environ(configuration, wsgi_url, method=None,
+        query=None, headers=None, form=None, mig_user_dn=None):
     """Populate the necessary variables that will constitute a valid WSGI
     environment given a URL to which we will make a requests under test and
     various other options that set up the nature of that request."""
@@ -67,7 +107,7 @@ def create_wsgi_environ(configuration, wsgi_url, method='GET', query=None, heade
         method = 'POST'
         request_query = ''
 
-        body = urlencode(MultiDict(form)).encode('ascii')
+        body = _urlencode_form(form)
 
         headers = headers or {}
         if not 'Content-Type' in headers:
@@ -76,6 +116,7 @@ def create_wsgi_environ(configuration, wsgi_url, method='GET', query=None, heade
         headers['Content-Length'] = str(len(body))
         wsgi_input = BytesIO(body)
     else:
+        assert method is not None, "method required with no payload specified"
         request_query = parsed_url.query
         wsgi_input = ()
 
@@ -95,10 +136,21 @@ def close(self, *ars, **kwargs):
     environ['HTTP_HOST'] = parsed_url.netloc
     environ['PATH_INFO'] = parsed_url.path
     environ['QUERY_STRING'] = request_query
+    environ['REMOTE_ADDR'] = '127.0.0.1'
     environ['REQUEST_METHOD'] = method
     environ['SCRIPT_URI'] = ''.join(
         ('http://', environ['HTTP_HOST'], environ['PATH_INFO']))
 
+    if mig_user_dn:
+        environ['REMOTE_USER'] = mig_user_dn
+
+    path_parts = parsed_url.path.split('/')
+    maybe_script_name = path_parts[-1]
+    _, script_ext = os.path.splitext(path_parts[-1])
+    if script_ext != '':
+        # the script has an extension, so treat it as a functionality file
+        environ['SCRIPT_NAME'] = maybe_script_name
+
     if headers:
         for k, v in headers.items():
             header_key = k.replace('-', '_').upper()
@@ -112,38 +164,72 @@ def close(self, *ars, **kwargs):
     return environ
 
 
-def create_wsgi_start_response():
-    return FakeWsgiStartResponse()
+class _PreparedWsgi:
+    """
+    Object representing a simulated WSGI request to be exercised by a test case.
+    """
 
+    def __init__(self, configuration, url, **kwargs):
+        self.configuration = configuration
+        self.environ = create_wsgi_environ(configuration, url, **kwargs)
+        self.start_response = FakeWsgiStartResponse()
 
-def prepare_wsgi(configuration, url, **kwargs):
-    return _PreparedWsgi(
-        create_wsgi_environ(configuration, url, **kwargs),
-        create_wsgi_start_response()
-    )
+    def __iter__(self):
+        return iter((self.environ, self.start_response))
 
+    def _bind_invocation(self):
+        self.application_args = (
+            self.environ,
+            self.start_response,
+        )
 
-def _trigger_and_unpack_result(wsgi_result):
-    chunks = list(wsgi_result)
-    assert len(chunks) > 0, "invocation returned no output"
-    complete_value = b''.join(chunks)
-    decoded_value = codecs.decode(complete_value, 'utf8')
-    return decoded_value
+        self.application_kwargs = dict(
+            configuration=self.configuration,
+            _set_os_environ=False,
+        )
+
+        return migwsgi.application(
+            *self.application_args,
+            **self.application_kwargs
+        )
+
+    @staticmethod
+    def trigger_wsgi(wsgi_result):
+        chunks = list(wsgi_result)
+        assert len(chunks) > 0, "invocation returned no output"
+        return b''.join(chunks)
+
+
+def prepare_wsgi(configuration, url, **kwargs):
+    if 'method' not in kwargs:
+        kwargs['method'] = 'GET'
+    return _PreparedWsgi(configuration, url, **kwargs)
 
 
 class WsgiAssertMixin:
     """Custom assertions for verifying server code executed under test."""
 
-    def assertWsgiResponse(self, wsgi_result, fake_wsgi, expected_status_code):
-        assert isinstance(fake_wsgi, _PreparedWsgi)
+    def prepareWsgiAssert(self, configuration, url, **kwargs):
+        return _PreparedWsgi(configuration, url, **kwargs)
 
-        content = _trigger_and_unpack_result(wsgi_result)
+    def assertWsgiResponse(self, wsgi_result, prepared_wsgi,
+                                        expected_status_code=None,
+                                        expected_content_type=None,
+                                        content_format=None):
+        assert isinstance(prepared_wsgi, _PreparedWsgi)
+
+        if wsgi_result:
+            # legacy codepath
+            pass
+        else:
+            wsgi_result = prepared_wsgi._bind_invocation()
+        content = _PreparedWsgi.trigger_wsgi(wsgi_result)
 
         def called_once(fake):
             assert hasattr(fake, 'calls')
             return len(fake.calls) == 1
 
-        fake_start_response = fake_wsgi.start_response
+        fake_start_response = prepared_wsgi.start_response
 
         try:
             self.assertTrue(called_once(fake_start_response))
@@ -155,11 +241,24 @@ def called_once(fake):
 
         wsgi_call = fake_start_response.calls[0]
 
-        # check for expected HTTP status code
-        wsgi_status = wsgi_call[0]
-        actual_status_code = int(wsgi_status[0:3])
-        self.assertEqual(actual_status_code, expected_status_code)
+        actual_content_type = wsgi_call
+        is_textual = None
+
+        if expected_status_code:
+            # check for expected HTTP status code
+            wsgi_status = wsgi_call[0]
+            actual_status_code = int(wsgi_status[0:3])
+            self.assertEqual(actual_status_code, expected_status_code)
 
         headers = dict(wsgi_call[1])
 
+        actual_content_type = headers.get('Content-Type', 'none/none')
+        if expected_content_type:
+            self.assertEqual(actual_content_type, expected_content_type, "mismatched Content-Type")
+
+        content_is_textual = actual_content_type in TEXTUAL_CONTENT_TYPES
+        if content_is_textual:
+            textual_content = codecs.decode(content, 'utf8')
+            return textual_content, headers
+
         return content, headers