From 8dbe013e65134001bd48eb54935beb71d7333a37 Mon Sep 17 00:00:00 2001 From: dobesv Date: Thu, 27 Feb 2014 16:22:36 -0800 Subject: Don't incorrectly default charset on FileResponse By passing the content_type into the constructor to Response, we can allow it to decide intelligently whether the default charset should apply. Otherwise we'd have to replicate that logic somehow, or live with weird charset annotations on images, pdfs, and zips. --- pyramid/response.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/pyramid/response.py b/pyramid/response.py index 0f61af472..ff1dd25d5 100644 --- a/pyramid/response.py +++ b/pyramid/response.py @@ -28,6 +28,14 @@ _BLOCK_SIZE = 4096 * 64 # 256K class Response(_Response): pass +def maybe_guess_mimetype(content_type, content_encoding, path): + if content_type is None: + content_type, content_encoding = mimetypes.guess_type(path, + strict=False) + if content_type is None: + content_type = 'application/octet-stream' + return dict(content_type=content_type, content_encoding=content_encoding) + class FileResponse(Response): """ A Response object that can be used to serve a static file from disk @@ -52,15 +60,8 @@ class FileResponse(Response): """ def __init__(self, path, request=None, cache_max_age=None, content_type=None, content_encoding=None): - super(FileResponse, self).__init__(conditional_response=True) + super(FileResponse, self).__init__(conditional_response=True, **maybe_guess_mimetype(content_type, content_encoding, path)) self.last_modified = getmtime(path) - if content_type is None: - content_type, content_encoding = mimetypes.guess_type(path, - strict=False) - if content_type is None: - content_type = 'application/octet-stream' - self.content_type = content_type - self.content_encoding = content_encoding content_length = getsize(path) f = open(path, 'rb') app_iter = None -- cgit v1.2.3 From 4823d8577f3dd09f908ed8e4381b4050400022db Mon Sep 17 00:00:00 2001 From: Dobes Vandermeer Date: Thu, 6 Mar 2014 19:34:29 -0800 Subject: Add tests for the content_type fix. --- pyramid/response.py | 18 +++++++++--------- pyramid/tests/fixtures/minimal.pdf | Bin 0 -> 1054 bytes pyramid/tests/fixtures/minimal.xml | 1 + pyramid/tests/test_response.py | 23 +++++++++++++++++------ 4 files changed, 27 insertions(+), 15 deletions(-) create mode 100755 pyramid/tests/fixtures/minimal.pdf create mode 100644 pyramid/tests/fixtures/minimal.xml diff --git a/pyramid/response.py b/pyramid/response.py index ff1dd25d5..6b5f0a561 100644 --- a/pyramid/response.py +++ b/pyramid/response.py @@ -28,14 +28,6 @@ _BLOCK_SIZE = 4096 * 64 # 256K class Response(_Response): pass -def maybe_guess_mimetype(content_type, content_encoding, path): - if content_type is None: - content_type, content_encoding = mimetypes.guess_type(path, - strict=False) - if content_type is None: - content_type = 'application/octet-stream' - return dict(content_type=content_type, content_encoding=content_encoding) - class FileResponse(Response): """ A Response object that can be used to serve a static file from disk @@ -60,7 +52,15 @@ class FileResponse(Response): """ def __init__(self, path, request=None, cache_max_age=None, content_type=None, content_encoding=None): - super(FileResponse, self).__init__(conditional_response=True, **maybe_guess_mimetype(content_type, content_encoding, path)) + if content_type is None: + content_type, content_encoding = mimetypes.guess_type(path, strict=False) + if content_type is None: + content_type = 'application/octet-stream' + super(FileResponse, self).__init__( + conditional_response=True, + content_type=content_type, + content_encoding=content_encoding + ) self.last_modified = getmtime(path) content_length = getsize(path) f = open(path, 'rb') diff --git a/pyramid/tests/fixtures/minimal.pdf b/pyramid/tests/fixtures/minimal.pdf new file mode 100755 index 000000000..e267be996 Binary files /dev/null and b/pyramid/tests/fixtures/minimal.pdf differ diff --git a/pyramid/tests/fixtures/minimal.xml b/pyramid/tests/fixtures/minimal.xml new file mode 100644 index 000000000..1972c155d --- /dev/null +++ b/pyramid/tests/fixtures/minimal.xml @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/pyramid/tests/test_response.py b/pyramid/tests/test_response.py index e6d90f979..afd354d97 100644 --- a/pyramid/tests/test_response.py +++ b/pyramid/tests/test_response.py @@ -23,21 +23,32 @@ class TestFileResponse(unittest.TestCase): from pyramid.response import FileResponse return FileResponse(file, **kw) - def _getPath(self): + def _getPath(self, suffix='txt'): here = os.path.dirname(__file__) - return os.path.join(here, 'fixtures', 'minimal.txt') + return os.path.join(here, 'fixtures', 'minimal.%s'%(suffix,)) def test_with_content_type(self): path = self._getPath() r = self._makeOne(path, content_type='image/jpeg') self.assertEqual(r.content_type, 'image/jpeg') + self.assertEqual(r.headers.get('content-type'), 'image/jpeg') + path = self._getPath() + r = self._makeOne(path, content_type='application/xml') + self.assertEqual(r.content_type, 'application/xml') + self.assertEqual(r.headers['content-type'], 'application/xml; charset=UTF-8') r.app_iter.close() def test_without_content_type(self): - path = self._getPath() - r = self._makeOne(path) - self.assertEqual(r.content_type, 'text/plain') - r.app_iter.close() + for suffix, content_type in ( + ('txt', 'text/plain; charset=UTF-8'), + ('xml', 'application/xml; charset=UTF-8'), + ('pdf', 'application/pdf') + ): + path = self._getPath(suffix) + r = self._makeOne(path) + self.assertEqual(r.content_type, content_type.split(';')[0]) + self.assertEqual(r.headers['content-type'], content_type) + r.app_iter.close() class TestFileIter(unittest.TestCase): def _makeOne(self, file, block_size): -- cgit v1.2.3 From c8a58abb1ed5705defb7d7464e4f0d086fe13a21 Mon Sep 17 00:00:00 2001 From: Dobes Vandermeer Date: Thu, 6 Mar 2014 21:29:52 -0800 Subject: Expand tests - make sure pdf, jpeg types also are not annotated with a charset. --- pyramid/tests/fixtures/minimal.jpg | Bin 0 -> 631 bytes pyramid/tests/test_response.py | 16 +++++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 pyramid/tests/fixtures/minimal.jpg diff --git a/pyramid/tests/fixtures/minimal.jpg b/pyramid/tests/fixtures/minimal.jpg new file mode 100644 index 000000000..1cda9a53d Binary files /dev/null and b/pyramid/tests/fixtures/minimal.jpg differ diff --git a/pyramid/tests/test_response.py b/pyramid/tests/test_response.py index afd354d97..8061ecac5 100644 --- a/pyramid/tests/test_response.py +++ b/pyramid/tests/test_response.py @@ -27,17 +27,27 @@ class TestFileResponse(unittest.TestCase): here = os.path.dirname(__file__) return os.path.join(here, 'fixtures', 'minimal.%s'%(suffix,)) - def test_with_content_type(self): - path = self._getPath() + def test_with_image_content_type(self): + path = self._getPath('jpg') r = self._makeOne(path, content_type='image/jpeg') self.assertEqual(r.content_type, 'image/jpeg') - self.assertEqual(r.headers.get('content-type'), 'image/jpeg') + self.assertEqual(r.headers['content-type'], 'image/jpeg') path = self._getPath() + + def test_with_xml_content_type(self): + path = self._getPath('xml') r = self._makeOne(path, content_type='application/xml') self.assertEqual(r.content_type, 'application/xml') self.assertEqual(r.headers['content-type'], 'application/xml; charset=UTF-8') r.app_iter.close() + def test_with_pdf_content_type(self): + path = self._getPath('xml') + r = self._makeOne(path, content_type='application/pdf') + self.assertEqual(r.content_type, 'application/pdf') + self.assertEqual(r.headers['content-type'], 'application/pdf') + r.app_iter.close() + def test_without_content_type(self): for suffix, content_type in ( ('txt', 'text/plain; charset=UTF-8'), -- cgit v1.2.3 From 745edec7be7c4c4be56cbdf01ffe3543cb3348e2 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Fri, 16 May 2014 21:34:02 -0500 Subject: close resources and 80-char line width --- pyramid/response.py | 3 ++- pyramid/tests/test_response.py | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/pyramid/response.py b/pyramid/response.py index 6b5f0a561..adc903b44 100644 --- a/pyramid/response.py +++ b/pyramid/response.py @@ -53,7 +53,8 @@ class FileResponse(Response): def __init__(self, path, request=None, cache_max_age=None, content_type=None, content_encoding=None): if content_type is None: - content_type, content_encoding = mimetypes.guess_type(path, strict=False) + content_type, content_encoding = ( + mimetypes.guess_type(path, strict=False)) if content_type is None: content_type = 'application/octet-stream' super(FileResponse, self).__init__( diff --git a/pyramid/tests/test_response.py b/pyramid/tests/test_response.py index 8061ecac5..8731fa764 100644 --- a/pyramid/tests/test_response.py +++ b/pyramid/tests/test_response.py @@ -25,7 +25,7 @@ class TestFileResponse(unittest.TestCase): def _getPath(self, suffix='txt'): here = os.path.dirname(__file__) - return os.path.join(here, 'fixtures', 'minimal.%s'%(suffix,)) + return os.path.join(here, 'fixtures', 'minimal.%s' % (suffix,)) def test_with_image_content_type(self): path = self._getPath('jpg') @@ -33,12 +33,14 @@ class TestFileResponse(unittest.TestCase): self.assertEqual(r.content_type, 'image/jpeg') self.assertEqual(r.headers['content-type'], 'image/jpeg') path = self._getPath() + r.app_iter.close() def test_with_xml_content_type(self): path = self._getPath('xml') r = self._makeOne(path, content_type='application/xml') self.assertEqual(r.content_type, 'application/xml') - self.assertEqual(r.headers['content-type'], 'application/xml; charset=UTF-8') + self.assertEqual(r.headers['content-type'], + 'application/xml; charset=UTF-8') r.app_iter.close() def test_with_pdf_content_type(self): -- cgit v1.2.3