openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #22224
[Merge] lp:~felipe-q/openlp/fix_httprouter into lp:openlp
Felipe Polo-Wood has proposed merging lp:~felipe-q/openlp/fix_httprouter into lp:openlp.
Requested reviews:
Tim Bentley (trb143)
Raoul Snyman (raoul-snyman)
For more details, see:
https://code.launchpad.net/~felipe-q/openlp/fix_httprouter/+merge/195332
HttpRouter.serve_file was never sending the response headers back to the client (this can be seen by watching the network traffic on the request for http://localhost:4316). Forgiving client browsers were assuming the content-type from the extension.
Another small problem was that the headers were set twice in the case of a 404 (although this did not have any user impact).
The code was missing the send_response and the end_headers calls.
The new code breaks the detection of the type from the sending of the headers so the appropriate response headers are always sent, and in the right order.
--
https://code.launchpad.net/~felipe-q/openlp/fix_httprouter/+merge/195332
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/plugins/remotes/lib/httprouter.py'
--- openlp/plugins/remotes/lib/httprouter.py 2013-10-13 20:36:42 +0000
+++ openlp/plugins/remotes/lib/httprouter.py 2013-11-15 02:52:43 +0000
@@ -128,6 +128,15 @@
from openlp.core.lib import Registry, PluginStatus, StringContent, image_to_byte
log = logging.getLogger(__name__)
+FILE_TYPES = {
+ '.html': 'text/html',
+ '.css': 'text/css',
+ '.js': 'application/javascript',
+ '.jpg': 'image/jpeg',
+ '.gif': 'image/gif',
+ '.ico': 'image/x-icon',
+ '.png': 'image/png'
+}
class HttpRouter(object):
@@ -346,30 +355,13 @@
path = os.path.normpath(os.path.join(self.html_dir, file_name))
if not path.startswith(self.html_dir):
return self.do_not_found()
- ext = os.path.splitext(file_name)[1]
- html = None
- if ext == '.html':
- self.send_header('Content-type', 'text/html')
- variables = self.template_vars
- html = Template(filename=path, input_encoding='utf-8', output_encoding='utf-8').render(**variables)
- elif ext == '.css':
- self.send_header('Content-type', 'text/css')
- elif ext == '.js':
- self.send_header('Content-type', 'application/javascript')
- elif ext == '.jpg':
- self.send_header('Content-type', 'image/jpeg')
- elif ext == '.gif':
- self.send_header('Content-type', 'image/gif')
- elif ext == '.ico':
- self.send_header('Content-type', 'image/x-icon')
- elif ext == '.png':
- self.send_header('Content-type', 'image/png')
- else:
- self.send_header('Content-type', 'text/plain')
+ content = None
+ ext, content_type = self.get_content_type(path)
file_handle = None
try:
- if html:
- content = html
+ if ext == '.html':
+ variables = self.template_vars
+ content = Template(filename=path, input_encoding='utf-8', output_encoding='utf-8').render(**variables)
else:
file_handle = open(path, 'rb')
log.debug('Opened %s' % path)
@@ -380,8 +372,22 @@
finally:
if file_handle:
file_handle.close()
+ self.send_response(200)
+ self.send_header('Content-type', content_type)
+ self.end_headers()
return content
+ def get_content_type(self, file_name):
+ """
+ Examines the extension of the file and determines
+ what the content_type should be, defaults to text/plain
+ Returns the extension and the content_type
+ """
+ content_type = 'text/plain'
+ ext = os.path.splitext(file_name)[1]
+ content_type = FILE_TYPES.get(ext, 'text/plain')
+ return ext, content_type
+
def poll(self):
"""
Poll OpenLP to determine the current slide number and item name.
=== modified file 'tests/functional/openlp_plugins/remotes/test_remotetab.py'
--- tests/functional/openlp_plugins/remotes/test_remotetab.py 2013-10-13 20:36:42 +0000
+++ tests/functional/openlp_plugins/remotes/test_remotetab.py 2013-11-15 02:52:43 +0000
@@ -62,7 +62,7 @@
"""
Create the UI
"""
- fd, self.ini_file = mkstemp('.ini')
+ self.fd, self.ini_file = mkstemp('.ini')
Settings().set_filename(self.ini_file)
self.application = QtGui.QApplication.instance()
Settings().extend_default_settings(__default_settings__)
@@ -76,6 +76,7 @@
del self.application
del self.parent
del self.form
+ os.close(self.fd)
os.unlink(self.ini_file)
def get_ip_address_default_test(self):
=== modified file 'tests/functional/openlp_plugins/remotes/test_router.py'
--- tests/functional/openlp_plugins/remotes/test_router.py 2013-10-13 20:36:42 +0000
+++ tests/functional/openlp_plugins/remotes/test_router.py 2013-11-15 02:52:43 +0000
@@ -37,7 +37,8 @@
from openlp.core.common import Settings
from openlp.plugins.remotes.lib.httpserver import HttpRouter
-from tests.functional import MagicMock
+from tests.functional import MagicMock, patch
+from mock import mock_open
__default_settings__ = {
'remotes/twelve hour': True,
@@ -50,6 +51,7 @@
'remotes/ip address': '0.0.0.0'
}
+TEST_PATH = os.path.abspath(os.path.dirname(__file__))
class TestRouter(TestCase):
"""
@@ -59,7 +61,7 @@
"""
Create the UI
"""
- fd, self.ini_file = mkstemp('.ini')
+ self.fd, self.ini_file = mkstemp('.ini')
Settings().set_filename(self.ini_file)
self.application = QtGui.QApplication.instance()
Settings().extend_default_settings(__default_settings__)
@@ -70,6 +72,7 @@
Delete all the C++ objects at the end so that we don't have a segfault
"""
del self.application
+ os.close(self.fd)
os.unlink(self.ini_file)
def password_encrypter_test(self):
@@ -109,4 +112,63 @@
assert function['function'] == mocked_function, \
'The mocked function should match defined value.'
assert function['secure'] == False, \
- 'The mocked function should not require any security.'
\ No newline at end of file
+ 'The mocked function should not require any security.'
+
+ def get_content_type_test(self):
+ """
+ Test the get_content_type logic
+ """
+ # GIVEN: a set of files and their corresponding types
+ headers = [ ['test.html', 'text/html'], ['test.css', 'text/css'],
+ ['test.js', 'application/javascript'], ['test.jpg', 'image/jpeg'],
+ ['test.gif', 'image/gif'], ['test.ico', 'image/x-icon'],
+ ['test.png', 'image/png'], ['test.whatever', 'text/plain'],
+ ['test', 'text/plain'], ['', 'text/plain'],
+ [os.path.join(TEST_PATH,'test.html'), 'text/html']]
+ # WHEN: calling each file type
+ for header in headers:
+ ext, content_type = self.router.get_content_type(header[0])
+ # THEN: all types should match
+ self.assertEqual(content_type, header[1], 'Mismatch of content type')
+
+ def serve_file_without_params_test(self):
+ """
+ Test the serve_file method without params
+ """
+ # GIVEN: mocked environment
+ self.router.send_response = MagicMock()
+ self.router.send_header = MagicMock()
+ self.router.end_headers = MagicMock()
+ self.router.wfile = MagicMock()
+ self.router.html_dir = os.path.normpath('test/dir')
+ self.router.template_vars = MagicMock()
+ # WHEN: call serve_file with no file_name
+ self.router.serve_file()
+ # THEN: it should return a 404
+ self.router.send_response.assert_called_once_with(404)
+ self.router.send_header.assert_called_once_with('Content-type','text/html')
+ self.assertEqual(self.router.end_headers.call_count, 1,
+ 'end_headers called once')
+
+ def serve_file_with_valid_params_test(self):
+ """
+ Test the serve_file method with an existing file
+ """
+ # GIVEN: mocked environment
+ self.router.send_response = MagicMock()
+ self.router.send_header = MagicMock()
+ self.router.end_headers = MagicMock()
+ self.router.wfile = MagicMock()
+ self.router.html_dir = os.path.normpath('test/dir')
+ self.router.template_vars = MagicMock()
+ with patch('openlp.core.lib.os.path.exists') as mocked_exists, \
+ patch('builtins.open', mock_open(read_data='123')):
+ mocked_exists.return_value = True
+ # WHEN: call serve_file with an existing html file
+ self.router.serve_file(os.path.normpath('test/dir/test.html'))
+ # THEN: it should return a 200 and the file
+ self.router.send_response.assert_called_once_with(200)
+ self.router.send_header.assert_called_once_with(
+ 'Content-type','text/html')
+ self.assertEqual(self.router.end_headers.call_count, 1,
+ 'end_headers called once')
Follow ups