From 989f6b8843c949aa2ce3839a37399fcada442ac7 Mon Sep 17 00:00:00 2001 From: Joakim Uddholm Date: Sun, 12 Jun 2016 01:38:01 +0200 Subject: [PATCH 1/7] Test to show bug with is_gzipped and Content-Type: application/gzip;charset. --- tests/test_utils_gz.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/test_utils_gz.py b/tests/test_utils_gz.py index 8fb1e414d..e107615f3 100644 --- a/tests/test_utils_gz.py +++ b/tests/test_utils_gz.py @@ -1,7 +1,8 @@ import unittest from os.path import join -from scrapy.utils.gz import gunzip +from scrapy.utils.gz import gunzip, is_gzipped +from scrapy.http import Response, Headers from tests import tests_datadir SAMPLEDIR = join(tests_datadir, 'compressed') @@ -27,3 +28,16 @@ class GunzipTest(unittest.TestCase): with open(join(SAMPLEDIR, 'truncated-crc-error-short.gz'), 'rb') as f: text = gunzip(f.read()) assert text.endswith(b'') + + def test_is_gzipped(self): + hdrs = Headers({"Content-Type": "application/x-gzip"}) + r1 = Response("http://www.example.com", headers=hdrs) + self.assertTrue(is_gzipped(r1)) + r2 = Response("http://www.example.com") + self.assertTrue(not is_gzipped(r2)) + hdrs = Headers({"Content-Type": "application/javascript"}) + r3 = Response("http://www.example.com", headers=hdrs) + self.assertTrue(not is_gzipped(r3)) + hdrs = Headers({"Content-Type": "application/x-gzip;charset=utf-8"}) + r1 = Response("http://www.example.com", headers=hdrs) + self.assertTrue(is_gzipped(r1)) From db729f5b304212518e3208995f44e66e55af420e Mon Sep 17 00:00:00 2001 From: Joakim Uddholm Date: Sun, 12 Jun 2016 02:26:16 +0200 Subject: [PATCH 2/7] Suggested fix for is_gzipped --- scrapy/utils/gz.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/utils/gz.py b/scrapy/utils/gz.py index d035f9fdf..f174950a4 100644 --- a/scrapy/utils/gz.py +++ b/scrapy/utils/gz.py @@ -54,4 +54,4 @@ def gunzip(data): def is_gzipped(response): """Return True if the response is gzipped, or False otherwise""" ctype = response.headers.get('Content-Type', b'') - return ctype in (b'application/x-gzip', b'application/gzip') + return b'application/x-gzip' in ctype or b'application/gzip' in ctype From 2c98a88a0e584fff64eb70f48b869494eca1d7ae Mon Sep 17 00:00:00 2001 From: Joakim Uddholm Date: Sun, 12 Jun 2016 10:49:34 +0200 Subject: [PATCH 3/7] Separated tests based on case --- tests/test_utils_gz.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/tests/test_utils_gz.py b/tests/test_utils_gz.py index e107615f3..3648d5c43 100644 --- a/tests/test_utils_gz.py +++ b/tests/test_utils_gz.py @@ -29,15 +29,24 @@ class GunzipTest(unittest.TestCase): text = gunzip(f.read()) assert text.endswith(b'') - def test_is_gzipped(self): + def test_is_gzipped_right(self): hdrs = Headers({"Content-Type": "application/x-gzip"}) r1 = Response("http://www.example.com", headers=hdrs) self.assertTrue(is_gzipped(r1)) - r2 = Response("http://www.example.com") - self.assertTrue(not is_gzipped(r2)) + hdrs = Headers({"Content-Type": "application/gzip"}) + r1 = Response("http://www.example.com", headers=hdrs) + self.assertTrue(is_gzipped(r1)) + + def test_is_gzipped_empty(self): + r1 = Response("http://www.example.com") + self.assertTrue(not is_gzipped(r1)) + + def test_is_gzipped_wrong(self): hdrs = Headers({"Content-Type": "application/javascript"}) - r3 = Response("http://www.example.com", headers=hdrs) - self.assertTrue(not is_gzipped(r3)) + r1 = Response("http://www.example.com", headers=hdrs) + self.assertTrue(not is_gzipped(r1)) + + def test_is_gzipped_with_charset(self): hdrs = Headers({"Content-Type": "application/x-gzip;charset=utf-8"}) r1 = Response("http://www.example.com", headers=hdrs) self.assertTrue(is_gzipped(r1)) From 124e218a3b3d8c5c4f924da5eb8399b205349d19 Mon Sep 17 00:00:00 2001 From: Joakim Uddholm Date: Tue, 14 Jun 2016 14:22:18 +0200 Subject: [PATCH 4/7] Added new testcases suggested by @redapple. --- tests/test_utils_gz.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/tests/test_utils_gz.py b/tests/test_utils_gz.py index 3648d5c43..a9bd29bae 100644 --- a/tests/test_utils_gz.py +++ b/tests/test_utils_gz.py @@ -37,14 +37,28 @@ class GunzipTest(unittest.TestCase): r1 = Response("http://www.example.com", headers=hdrs) self.assertTrue(is_gzipped(r1)) + def test_is_gzipped_not_quite(self): + hdrs = Headers({"Content-Type": "application/gzippppp"}) + r1 = Response("http://www.example.com", headers=hdrs) + self.assertFalse(is_gzipped(r1)) + + def test_is_gzipped_case_insensitive(self): + hdrs = Headers({"Content-Type": "Application/X-Gzip"}) + r1 = Response("http://www.example.com", headers=hdrs) + self.assertTrue(is_gzipped(r1)) + + hdrs = Headers({"Content-Type": "application/X-GZIP ; charset=utf-8"}) + r1 = Response("http://www.example.com", headers=hdrs) + self.assertTrue(is_gzipped(r1)) + def test_is_gzipped_empty(self): r1 = Response("http://www.example.com") - self.assertTrue(not is_gzipped(r1)) + self.assertFalse(is_gzipped(r1)) def test_is_gzipped_wrong(self): hdrs = Headers({"Content-Type": "application/javascript"}) r1 = Response("http://www.example.com", headers=hdrs) - self.assertTrue(not is_gzipped(r1)) + self.assertFalse(is_gzipped(r1)) def test_is_gzipped_with_charset(self): hdrs = Headers({"Content-Type": "application/x-gzip;charset=utf-8"}) From 259426ec9995da9a5415de9d851febf788160cf3 Mon Sep 17 00:00:00 2001 From: Joakim Uddholm Date: Tue, 14 Jun 2016 14:39:16 +0200 Subject: [PATCH 5/7] is_gzipped: Changed to regex to check the content-type header. Also suggested by @redapple. --- scrapy/utils/gz.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scrapy/utils/gz.py b/scrapy/utils/gz.py index f174950a4..f2a9555b1 100644 --- a/scrapy/utils/gz.py +++ b/scrapy/utils/gz.py @@ -7,7 +7,7 @@ except ImportError: from gzip import GzipFile import six - +import re # - Python>=3.5 GzipFile's read() has issues returning leftover # uncompressed data when input is corrupted @@ -50,8 +50,9 @@ def gunzip(data): raise return output +_is_gzipped_re = re.compile(br'^application/(x-)?gzip\b', re.I) def is_gzipped(response): """Return True if the response is gzipped, or False otherwise""" ctype = response.headers.get('Content-Type', b'') - return b'application/x-gzip' in ctype or b'application/gzip' in ctype + return not _is_gzipped_re.search(ctype) is None From 36928d897ca44b0a62ebcb1c3fb358cbfd07440f Mon Sep 17 00:00:00 2001 From: Joakim Uddholm Date: Tue, 14 Jun 2016 15:40:20 +0200 Subject: [PATCH 6/7] is_gzipped: improved readability --- scrapy/utils/gz.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/utils/gz.py b/scrapy/utils/gz.py index f2a9555b1..cfb652143 100644 --- a/scrapy/utils/gz.py +++ b/scrapy/utils/gz.py @@ -55,4 +55,4 @@ _is_gzipped_re = re.compile(br'^application/(x-)?gzip\b', re.I) def is_gzipped(response): """Return True if the response is gzipped, or False otherwise""" ctype = response.headers.get('Content-Type', b'') - return not _is_gzipped_re.search(ctype) is None + return _is_gzipped_re.search(ctype) is not None From 23f99e98c4cf97891202847f3384a35b6fc57e7c Mon Sep 17 00:00:00 2001 From: Joakim Uddholm Date: Tue, 14 Jun 2016 21:33:51 +0200 Subject: [PATCH 7/7] is_gzipped: Separated tests again. --- tests/test_utils_gz.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_utils_gz.py b/tests/test_utils_gz.py index a9bd29bae..2b47bf8da 100644 --- a/tests/test_utils_gz.py +++ b/tests/test_utils_gz.py @@ -29,10 +29,12 @@ class GunzipTest(unittest.TestCase): text = gunzip(f.read()) assert text.endswith(b'') - def test_is_gzipped_right(self): + def test_is_x_gzipped_right(self): hdrs = Headers({"Content-Type": "application/x-gzip"}) r1 = Response("http://www.example.com", headers=hdrs) self.assertTrue(is_gzipped(r1)) + + def test_is_gzipped_right(self): hdrs = Headers({"Content-Type": "application/gzip"}) r1 = Response("http://www.example.com", headers=hdrs) self.assertTrue(is_gzipped(r1))