Skip to content

Commit 85113bb

Browse files
committed
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.
1 parent 5b8e95f commit 85113bb

4 files changed

Lines changed: 177 additions & 32 deletions

File tree

mig/shared/output.py

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
exit(1)
3939

4040
from past.builtins import basestring
41+
import inspect
4142
import os
4243
import sys
4344
import time
@@ -64,6 +65,20 @@
6465
'yaml', 'xmlrpc', 'resource', 'json', 'file']
6566

6667

68+
def kwargs_for_functionality(main, configuration=None, environ=None):
69+
"""
70+
Determine which additional arguments are supported by the
71+
selected functionality method and arrange to to pass them.
72+
"""
73+
74+
parameters = inspect.signature(main).parameters
75+
76+
kwargs = dict()
77+
if 'environ' in parameters:
78+
kwargs['environ'] = environ
79+
return kwargs
80+
81+
6782
def reject_main(client_id, user_arguments_dict):
6883
"""A simple main-function to use if functionality backend is disabled"""
6984
output_objs = [bailout_title(None, 'Access Error'),
@@ -2720,9 +2735,6 @@ def file_format(configuration, ret_val, ret_msg, out_obj):
27202735

27212736
# TODO: use wsgi file_wrapper helper here if out_obj has wsgi entry?
27222737

2723-
# NOTE: we expect binary data here and must use it consistently
2724-
file_content = b''
2725-
27262738
# NOTE: carefully handle errors and ONLY render them when proper care has
27272739
# been taken to deliver them as actual output, to avoid that they end
27282740
# up hidden inside downloaded files.
@@ -2742,6 +2754,16 @@ def file_format(configuration, ret_val, ret_msg, out_obj):
27422754
render_text, render_errors = True, True
27432755
# _logger.debug("render output in file_format: %s (%s %s)" %
27442756
# (out_obj, render_text, render_errors))
2757+
2758+
# determine whether the content is binary or not
2759+
should_treat_as_binary = any((entry['object_type'] == 'binary'
2760+
for entry in out_obj))
2761+
2762+
if should_treat_as_binary:
2763+
file_content = b''
2764+
else:
2765+
file_content = ''
2766+
27452767
for entry in out_obj:
27462768
if entry['object_type'] == 'file_output':
27472769
for line in entry['lines']:

mig/wsgi-bin/migwsgi.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@
4444
from mig.shared.defaults import download_block_size, default_fs_coding
4545
from mig.shared.conf import get_configuration_object
4646
from mig.shared.objecttypes import get_object_type_info
47-
from mig.shared.output import validate, format_output, dummy_main, reject_main
47+
from mig.shared.output import validate, format_output, \
48+
kwargs_for_functionality, dummy_main, reject_main
4849
from mig.shared.safeinput import valid_backend_name, html_escape, InputException
4950
from mig.shared.scriptinput import fieldstorage_to_dict, FixedFieldStorage
5051

@@ -124,8 +125,11 @@ def stub(configuration, client_id, import_path, backend, user_arguments_dict,
124125
return (output_objects, returnvalues.INVALID_ARGUMENT)
125126

126127
try:
128+
main_kwargs = kwargs_for_functionality(main,
129+
environ=environ)
127130
(output_objects, (ret_code, ret_msg)) = main(client_id,
128-
user_arguments_dict)
131+
user_arguments_dict,
132+
**main_kwargs)
129133
except Exception as err:
130134
import traceback
131135
_logger.error("%s script crashed:\n%s" % (_addr,

tests/support/wsgisupp.py

Lines changed: 117 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,31 @@
2929

3030
from collections import namedtuple
3131
import codecs
32+
import importlib
3233
from io import BytesIO
34+
import os
35+
import sys
3336
from urllib.parse import urlencode, urlparse
3437

35-
from werkzeug.datastructures import MultiDict
38+
from tests.support.suppconst import MIG_BASE
3639

40+
OBJECTS_TYPE = 'objects'
3741

38-
# named type representing the tuple that is passed to WSGI handlers
39-
_PreparedWsgi = namedtuple('_PreparedWsgi', ['environ', 'start_response'])
42+
43+
def _import_forcibly(module_name, relative_module_dir=None):
44+
"""Custom import function to allow an import of a file for testing
45+
that resides within a non-module directory."""
46+
47+
module_path = os.path.join(MIG_BASE, 'mig')
48+
if relative_module_dir is not None:
49+
module_path = os.path.join(module_path, relative_module_dir)
50+
sys.path.append(module_path)
51+
mod = importlib.import_module(module_name)
52+
sys.path.pop(-1) # do not leave the forced module path
53+
return mod
54+
55+
56+
migwsgi = _import_forcibly('migwsgi', relative_module_dir='wsgi-bin')
4057

4158

4259
class FakeWsgiStartResponse:
@@ -51,7 +68,29 @@ def __call__(self, status, headers, exc=None):
5168
self.calls.append((status, headers, exc))
5269

5370

54-
def create_wsgi_environ(configuration, wsgi_url, method='GET', query=None, headers=None, form=None):
71+
def _urlencode_form(form_content):
72+
"""
73+
Convert a data structure describing form contents to byte string
74+
that can be directly sent as the body of an HTTP request.
75+
"""
76+
77+
field_key_and_value_pairs = []
78+
if isinstance(form_content, dict):
79+
for key, value in form_content.items():
80+
if isinstance(value, list):
81+
for item in value:
82+
field_key_and_value_pairs.append((key, item))
83+
continue
84+
field_key_and_value_pairs.append((key, value))
85+
elif isinstance(form_content, list):
86+
field_key_and_value_pairs = form_content
87+
else:
88+
raise AssertionError("invalid form content")
89+
return urlencode(field_key_and_value_pairs, doseq=True).encode('ascii')
90+
91+
92+
def create_wsgi_environ(configuration, wsgi_url, method=None,
93+
query=None, headers=None, form=None, mig_user_dn=None):
5594
"""Populate the necessary variables that will constitute a valid WSGI
5695
environment given a URL to which we will make a requests under test and
5796
various other options that set up the nature of that request."""
@@ -67,7 +106,7 @@ def create_wsgi_environ(configuration, wsgi_url, method='GET', query=None, heade
67106
method = 'POST'
68107
request_query = ''
69108

70-
body = urlencode(MultiDict(form)).encode('ascii')
109+
body = _urlencode_form(form)
71110

72111
headers = headers or {}
73112
if not 'Content-Type' in headers:
@@ -76,6 +115,7 @@ def create_wsgi_environ(configuration, wsgi_url, method='GET', query=None, heade
76115
headers['Content-Length'] = str(len(body))
77116
wsgi_input = BytesIO(body)
78117
else:
118+
assert method is not None, "method required with no payload specified"
79119
request_query = parsed_url.query
80120
wsgi_input = ()
81121

@@ -99,6 +139,16 @@ def close(self, *ars, **kwargs):
99139
environ['SCRIPT_URI'] = ''.join(
100140
('http://', environ['HTTP_HOST'], environ['PATH_INFO']))
101141

142+
if mig_user_dn:
143+
environ['REMOTE_USER'] = mig_user_dn
144+
145+
path_parts = parsed_url.path.split('/')
146+
maybe_script_name = path_parts[-1]
147+
_, script_ext = os.path.splitext(path_parts[-1])
148+
if script_ext != '':
149+
# the script has an extension, so treat it as a functionality file
150+
environ['SCRIPT_NAME'] = maybe_script_name
151+
102152
if headers:
103153
for k, v in headers.items():
104154
header_key = k.replace('-', '_').upper()
@@ -112,38 +162,74 @@ def close(self, *ars, **kwargs):
112162
return environ
113163

114164

115-
def create_wsgi_start_response():
116-
return FakeWsgiStartResponse()
165+
class _PreparedWsgi:
166+
"""
167+
Object representing a simulated WSGI request to be exercised by a test case.
168+
"""
117169

170+
def __init__(self, configuration, url, **kwargs):
171+
self.configuration = configuration
172+
self.environ = create_wsgi_environ(configuration, url, **kwargs)
173+
self.start_response = FakeWsgiStartResponse()
118174

119-
def prepare_wsgi(configuration, url, **kwargs):
120-
return _PreparedWsgi(
121-
create_wsgi_environ(configuration, url, **kwargs),
122-
create_wsgi_start_response()
123-
)
175+
def __iter__(self):
176+
return iter((self.environ, self.start_response))
177+
178+
def _bind_invocation(self):
179+
self.application_args = (
180+
self.environ,
181+
self.start_response,
182+
)
183+
184+
self.application_kwargs = dict(
185+
configuration=self.configuration,
186+
_set_os_environ=False,
187+
)
124188

189+
return migwsgi.application(
190+
*self.application_args,
191+
**self.application_kwargs
192+
)
125193

126-
def _trigger_and_unpack_result(wsgi_result):
127-
chunks = list(wsgi_result)
128-
assert len(chunks) > 0, "invocation returned no output"
129-
complete_value = b''.join(chunks)
130-
decoded_value = codecs.decode(complete_value, 'utf8')
131-
return decoded_value
194+
@staticmethod
195+
def trigger_and_unpack_result(wsgi_result):
196+
chunks = list(wsgi_result)
197+
assert len(chunks) > 0, "invocation returned no output"
198+
complete_value = b''.join(chunks)
199+
decoded_value = codecs.decode(complete_value, 'utf8')
200+
return decoded_value
201+
202+
203+
def prepare_wsgi(configuration, url, **kwargs):
204+
if 'method' not in kwargs:
205+
kwargs['method'] = 'GET'
206+
return _PreparedWsgi(configuration, url, **kwargs)
132207

133208

134209
class WsgiAssertMixin:
135210
"""Custom assertions for verifying server code executed under test."""
136211

137-
def assertWsgiResponse(self, wsgi_result, fake_wsgi, expected_status_code):
138-
assert isinstance(fake_wsgi, _PreparedWsgi)
212+
def prepareWsgiAssert(self, configuration, url, **kwargs):
213+
return _PreparedWsgi(configuration, url, **kwargs)
214+
215+
def assertWsgiResponse(self, wsgi_result, prepared_wsgi,
216+
expected_status_code=None,
217+
expected_content_type=None,
218+
content_format=None):
219+
assert isinstance(prepared_wsgi, _PreparedWsgi)
139220

140-
content = _trigger_and_unpack_result(wsgi_result)
221+
if wsgi_result:
222+
# legacy codepath
223+
pass
224+
else:
225+
wsgi_result = prepared_wsgi._bind_invocation()
226+
content = _PreparedWsgi.trigger_and_unpack_result(wsgi_result)
141227

142228
def called_once(fake):
143229
assert hasattr(fake, 'calls')
144230
return len(fake.calls) == 1
145231

146-
fake_start_response = fake_wsgi.start_response
232+
fake_start_response = prepared_wsgi.start_response
147233

148234
try:
149235
self.assertTrue(called_once(fake_start_response))
@@ -155,11 +241,16 @@ def called_once(fake):
155241

156242
wsgi_call = fake_start_response.calls[0]
157243

158-
# check for expected HTTP status code
159-
wsgi_status = wsgi_call[0]
160-
actual_status_code = int(wsgi_status[0:3])
161-
self.assertEqual(actual_status_code, expected_status_code)
244+
if expected_status_code:
245+
# check for expected HTTP status code
246+
wsgi_status = wsgi_call[0]
247+
actual_status_code = int(wsgi_status[0:3])
248+
self.assertEqual(actual_status_code, expected_status_code)
162249

163250
headers = dict(wsgi_call[1])
164251

252+
if expected_content_type:
253+
actual_content_type = headers.get('Content-Type', 'none/none')
254+
self.assertEqual(actual_content_type, expected_content_type, "mismatched Content-Type")
255+
165256
return content, headers

tests/test_mig_shared_functionality_cat.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636

3737
from tests.support import MIG_BASE, PY2, TEST_DATA_DIR, MigTestCase, testmain, \
3838
temppath, ensure_dirs_exist
39+
from tests.support.wsgisupp import WsgiAssertMixin
3940

4041
from mig.shared.base import client_id_dir
4142
from mig.shared.functionality.cat import _main as submain, main as realmain
@@ -60,7 +61,7 @@ def _only_output_objects(output_objects, with_object_type=None):
6061
return [o for o in output_objects if o['object_type'] == with_object_type]
6162

6263

63-
class MigSharedFunctionalityCat(MigTestCase):
64+
class MigSharedFunctionalityCat__submain(MigTestCase):
6465
"""Wrap unit tests for the corresponding module"""
6566

6667
TEST_CLIENT_ID = '/C=DK/ST=NA/L=NA/O=Test Org/OU=NA/CN=Test User/emailAddress=test@example.com'
@@ -199,5 +200,32 @@ def test_main_passes_environ(self):
199200
relevant_obj['text'], 'Input arguments were rejected - not allowed for this script!')
200201

201202

203+
class MigSharedFunctionalityCat__wsgi(MigTestCase, WsgiAssertMixin):
204+
205+
TEST_CLIENT_ID = '/C=DK/ST=NA/L=NA/O=Test Org/OU=NA/CN=Test User/emailAddress=test@example.com'
206+
207+
def _provide_configuration(self):
208+
return 'testconfig'
209+
210+
def before_each(self):
211+
self.test_user_dir = self._provision_test_user(self, self.TEST_CLIENT_ID)
212+
213+
def test_should_return_(self):
214+
request_body = {
215+
'path': 'nonexist.ent',
216+
'output_format': 'file',
217+
}
218+
prepared_wsgi = self.prepareWsgiAssert(self.configuration,
219+
'http://localhost/cat.py',
220+
form=request_body,
221+
mig_user_dn=self.TEST_CLIENT_ID)
222+
223+
content, _ = self.assertWsgiResponse(None, prepared_wsgi,
224+
expected_status_code=200,
225+
expected_content_type='text/plain')
226+
227+
self.assertEqual(content, 'nonexist.ent: No such file or directory\n')
228+
229+
202230
if __name__ == '__main__':
203231
testmain()

0 commit comments

Comments
 (0)