From 592fd71028f949a3a2eb1d908441d6acafee8970 Mon Sep 17 00:00:00 2001 From: dobesv Date: Thu, 27 Feb 2014 16:22:36 -0800 Subject: [PATCH 1/3] 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 0f61af472b..ff1dd25d50 100644 --- a/pyramid/response.py +++ b/pyramid/response.py @@ -28,6 +28,14 @@ def init_mimetypes(mimetypes): 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 From 92d53f27569f04bbde44a2dc5334a00f0a2868cd Mon Sep 17 00:00:00 2001 From: Dobes Vandermeer Date: Thu, 6 Mar 2014 19:34:29 -0800 Subject: [PATCH 2/3] 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 ff1dd25d50..6b5f0a5611 100644 --- a/pyramid/response.py +++ b/pyramid/response.py @@ -28,14 +28,6 @@ def init_mimetypes(mimetypes): 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 0000000000000000000000000000000000000000..e267be996e5e1ea9a26e8bacd5bc33014be6bbfd GIT binary patch literal 1054 zcma)5O=uHA6c%f-Y^9)7L817FM*4%W9Q1GTy4`NF}q*9?hIJ0TerCuDCot-VU z(Y=yK7Tn|W&z1x~c){#CCxk*E9>$pz?S@7MB?JHvqehxy!saxhID$1($3kq; zJ*#`0-t;eADx2E;-Zk-LjH=ttX7>!%B>OwxEe;)3r>n2Rm$qwo=cDJZcE1?Da_aI# zynXcZ)KjEXjGnCe^9Gd*P&@?AKLhaYi4!x31xBecA7tt<|xYk8k(s zm47a}zO~GiM|b=v|8#wSyMQzL3TO7k)pCh^W+J6Pkni)AuT%?LeQTb^Iw(^Pp%4ey z5RsQHKnx!Q2}BLCH>(HRU~l%UN$;d \ No newline at end of file diff --git a/pyramid/tests/test_response.py b/pyramid/tests/test_response.py index e6d90f979b..afd354d979 100644 --- a/pyramid/tests/test_response.py +++ b/pyramid/tests/test_response.py @@ -23,21 +23,32 @@ def _makeOne(self, file, **kw): 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): From 06c127ecf854ae64dbd062b355955479a9e819b1 Mon Sep 17 00:00:00 2001 From: Dobes Vandermeer Date: Thu, 6 Mar 2014 21:29:52 -0800 Subject: [PATCH 3/3] 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 0000000000000000000000000000000000000000..1cda9a53dc357ce07d3c67051b7615ebf7dc2f64 GIT binary patch literal 631 zcmex=^(PF6}rMnOeST|r4lSw=>~TvNxu(8R<ECr+Na zbot8FYu9hwy!G(W<0ns_J%91?)yGetzkL1n{m0K=Ab&A3Fhjfr_ZgbM1cClyVqsxs zVF&q(k*OSrnFU!`6%E;h90S=C3x$=88aYIqCNA7~kW<+>=!0ld(M2vX6_bamA3L7B$%azX<@d&d)*s literal 0 HcmV?d00001 diff --git a/pyramid/tests/test_response.py b/pyramid/tests/test_response.py index afd354d979..8061ecac52 100644 --- a/pyramid/tests/test_response.py +++ b/pyramid/tests/test_response.py @@ -27,17 +27,27 @@ def _getPath(self, suffix='txt'): 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'),