Skip to content

Commit 5089682

Browse files
committed
Make responding with binary data work under PY3.
The code as previously written would unconditionally encode parts as though they contained text - this went unnoticed on PY2 because strings and bytes are one and the same thing but blew up on PY3 where a string is explicitly of type unicode while a binary file would be raw bytes. Explicitly check the output_format and if instructed to serve a file do so without touching the chunks of file content bytes being yielded.
1 parent acb2cc2 commit 5089682

File tree

2 files changed

+73
-6
lines changed

2 files changed

+73
-6
lines changed

mig/wsgi-bin/migwsgi.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ def _set_os_environ(value):
206206
return _application(None, environ, start_response, _set_environ=_set_os_environ, _wrap_wsgi_errors=wrap_wsgi_errors)
207207

208208

209-
def _application(configuration, environ, start_response, _set_environ, _format_output=format_output, _retrieve_handler=_import_backend, _wrap_wsgi_errors=True, _config_file=None, _skip_log=False):
209+
def _application(configuration, environ, start_response, _set_environ, _fieldstorage_to_dict=fieldstorage_to_dict, _format_output=format_output, _retrieve_handler=_import_backend, _wrap_wsgi_errors=True, _config_file=None, _skip_log=False):
210210

211211
# NOTE: pass app environ including apache and query args on to sub handlers
212212
# through the usual 'os.environ' channel expected in functionality
@@ -319,7 +319,7 @@ def _application(configuration, environ, start_response, _set_environ, _format_o
319319
# (backend, script_name))
320320
fieldstorage = cgi.FieldStorage(fp=environ['wsgi.input'],
321321
environ=environ)
322-
user_arguments_dict = fieldstorage_to_dict(fieldstorage)
322+
user_arguments_dict = _fieldstorage_to_dict(fieldstorage)
323323
if 'output_format' in user_arguments_dict:
324324
output_format = user_arguments_dict['output_format'][0]
325325

@@ -437,7 +437,10 @@ def _application(configuration, environ, start_response, _set_environ, _format_o
437437
# (backend, i+1, chunk_parts))
438438
# end index may be after end of content - but no problem
439439
part = output[i*download_block_size:(i+1)*download_block_size]
440-
yield force_utf8(part)
440+
if output_format == 'file':
441+
yield part
442+
else:
443+
yield force_utf8(part)
441444
if chunk_parts > 1:
442445
_logger.info("WSGI %s finished yielding all %d output parts" %
443446
(backend, chunk_parts))

tests/test_mig_wsgi-bin_migwsgi.py

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@
3232
import os
3333
import stat
3434
import sys
35+
import unittest
3536

36-
from tests.support import MIG_BASE, MigTestCase, testmain
37+
from tests.support import MIG_BASE, TEST_BASE, TEST_DATA_DIR, MigTestCase, testmain
3738
from mig.shared.output import format_output
3839
import mig.shared.returnvalues as returnvalues
3940

@@ -65,14 +66,35 @@ def _is_return_value(return_value):
6566
return return_value in defined_return_values
6667

6768

68-
def _trigger_and_unpack_result(application_result):
69+
def _trigger_and_unpack_result(application_result, result_kind='textual'):
70+
assert result_kind in ('textual', 'binary')
71+
6972
chunks = list(application_result)
7073
assert len(chunks) > 0, "invocation returned no output"
7174
complete_value = b''.join(chunks)
72-
decoded_value = codecs.decode(complete_value, 'utf8')
75+
if result_kind == 'binary':
76+
decoded_value = complete_value
77+
else:
78+
decoded_value = codecs.decode(complete_value, 'utf8')
7379
return decoded_value
7480

7581

82+
def create_instrumented_fieldstorage_to_dict():
83+
def _instrumented_fieldstorage_to_dict(fieldstorage):
84+
return _instrumented_fieldstorage_to_dict._result
85+
86+
_instrumented_fieldstorage_to_dict._result = {
87+
'output_format': ('html',)
88+
}
89+
90+
def set_result(result):
91+
_instrumented_fieldstorage_to_dict._result = result
92+
93+
_instrumented_fieldstorage_to_dict.set_result = set_result
94+
95+
return _instrumented_fieldstorage_to_dict
96+
97+
7698
def create_instrumented_format_output():
7799
def _instrumented_format_output(
78100
configuration,
@@ -87,6 +109,16 @@ def _instrumented_format_output(
87109
call_args = (configuration, backend, ret_val, ret_msg, call_args_out_obj, outputformat,)
88110
_instrumented_format_output.calls.append({ 'args': call_args })
89111

112+
if _instrumented_format_output._file:
113+
return format_output(
114+
configuration,
115+
backend,
116+
ret_val,
117+
ret_msg,
118+
out_obj,
119+
outputformat,
120+
)
121+
90122
# FIXME: the following is a workaround for a bug that exists between the WSGI wrapper
91123
# and the output formatter - specifically, the latter adds default header and
92124
# title if start does not exist, but the former ensures that start always exists
@@ -121,11 +153,18 @@ def _instrumented_format_output(
121153
outputformat,
122154
)
123155
_instrumented_format_output.calls = []
156+
_instrumented_format_output._file = False
124157
_instrumented_format_output.values = dict(
125158
title_text='',
126159
header_text='',
127160
)
128161

162+
163+
def _set_file(is_enabled):
164+
_instrumented_format_output._file = is_enabled
165+
166+
setattr(_instrumented_format_output, 'set_file', _set_file)
167+
129168
def _program_values(**kwargs):
130169
_instrumented_format_output.values.update(kwargs)
131170

@@ -179,6 +218,7 @@ def before_each(self):
179218
self.fake_wsgi = prepare_wsgi(self.configuration, 'http://localhost/')
180219

181220
# MiG WSGI wrapper specific setup
221+
self.instrumented_fieldstorage_to_dict = create_instrumented_fieldstorage_to_dict()
182222
self.instrumented_format_output = create_instrumented_format_output()
183223
self.instrumented_retrieve_handler = create_instrumented_retrieve_handler()
184224

@@ -190,6 +230,7 @@ def before_each(self):
190230
self.application_kwargs = dict(
191231
_wrap_wsgi_errors=noop,
192232
_format_output=self.instrumented_format_output,
233+
_fieldstorage_to_dict=self.instrumented_fieldstorage_to_dict,
193234
_retrieve_handler=self.instrumented_retrieve_handler,
194235
_set_environ=noop,
195236
)
@@ -230,6 +271,29 @@ def test_return_value_ok_returns_expected_title(self):
230271
self.assertHtmlElementTextContent(output, 'title', 'TEST', trim_newlines=True)
231272
self.assertInstrumentation()
232273

274+
def test_return_value_ok_serving_a_binary_file(self):
275+
test_binary_file = os.path.join(TEST_DATA_DIR, 'loading.gif')
276+
with open(test_binary_file, 'rb') as f:
277+
test_binary_data = f.read()
278+
279+
self.instrumented_fieldstorage_to_dict.set_result({
280+
'output_format': ('file',)
281+
})
282+
self.instrumented_format_output.set_file(True)
283+
284+
file_obj = { 'object_type': 'binary', 'data': test_binary_data }
285+
self.instrumented_retrieve_handler.program([file_obj], returnvalues.OK)
286+
287+
application_result = migwsgi._application(
288+
*self.application_args,
289+
**self.application_kwargs
290+
)
291+
292+
output = _trigger_and_unpack_result(application_result, 'binary')
293+
294+
self.assertInstrumentation()
295+
self.assertEqual(output, test_binary_data)
296+
233297

234298
if __name__ == '__main__':
235299
testmain()

0 commit comments

Comments
 (0)