Merge pull request #2391 from redapple/always-gunzip-content-enc-gzip

[MRG] Always decompress Content-Encoding: gzip at HttpCompression stage

scrapy/downloadermiddlewares/httpcompression.py

@@ -1,6 +1,6 @@
 import zlib
 
-from scrapy.utils.gz import gunzip, is_gzipped
+from scrapy.utils.gz import gunzip
 from scrapy.http import Response, TextResponse
 from scrapy.responsetypes import responsetypes
 from scrapy.exceptions import NotConfigured
@@ -34,7 +34,7 @@ class HttpCompressionMiddleware(object):
             return response
         if isinstance(response, Response):
             content_encoding = response.headers.getlist('Content-Encoding')
-            if content_encoding and not is_gzipped(response):
+            if content_encoding:
                 encoding = content_encoding.pop()
                 decoded_body = self._decode(response.body, encoding.lower())
                 respcls = responsetypes.from_args(headers=response.headers, \

scrapy/spiders/sitemap.py

@@ -5,7 +5,8 @@ import six
 from scrapy.spiders import Spider
 from scrapy.http import Request, XmlResponse
 from scrapy.utils.sitemap import Sitemap, sitemap_urls_from_robots
-from scrapy.utils.gz import gunzip, is_gzipped
+from scrapy.utils.gz import gunzip, gzip_magic_number
+
 
 logger = logging.getLogger(__name__)
 
@@ -59,12 +60,19 @@ class SitemapSpider(Spider):
         """
         if isinstance(response, XmlResponse):
             return response.body
-        elif is_gzipped(response):
+        elif gzip_magic_number(response):
             return gunzip(response.body)
-        elif response.url.endswith('.xml'):
+        # actual gzipped sitemap files are decompressed above ;
+        # if we are here (response body is not gzipped)
+        # and have a response for .xml.gz,
+        # it usually means that it was already gunzipped
+        # by HttpCompression middleware,
+        # the HTTP response being sent with "Content-Encoding: gzip"
+        # without actually being a .xml.gz file in the first place,
+        # merely XML gzip-compressed on the fly,
+        # in other word, here, we have plain XML
+        elif response.url.endswith('.xml') or response.url.endswith('.xml.gz'):
             return response.body
-        elif response.url.endswith('.xml.gz'):
-            return gunzip(response.body)
 
 
 def regex(x):

scrapy/utils/gz.py

@@ -59,3 +59,7 @@ def is_gzipped(response):
     cenc = response.headers.get('Content-Encoding', b'').lower()
     return (_is_gzipped(ctype) or
             (_is_octetstream(ctype) and cenc in (b'gzip', b'x-gzip')))
+
+
+def gzip_magic_number(response):
+    return response.body[:3] == b'\x1f\x8b\x08'

tests/test_downloadermiddleware_httpcompression.py

@@ -8,6 +8,7 @@ from scrapy.http import Response, Request, HtmlResponse
 from scrapy.downloadermiddlewares.httpcompression import HttpCompressionMiddleware, \
     ACCEPTED_ENCODINGS
 from scrapy.responsetypes import responsetypes
+from scrapy.utils.gz import gunzip
 from tests import tests_datadir
 from w3lib.encoding import resolve_encoding
 
@@ -173,9 +174,9 @@ class HttpCompressionTest(TestCase):
         request = response.request
 
         newresponse = self.mw.process_response(request, response, self.spider)
-        self.assertIs(newresponse, response)
-        self.assertEqual(response.headers['Content-Encoding'], b'gzip')
-        self.assertEqual(response.headers['Content-Type'], b'application/gzip')
+        self.assertIsNot(newresponse, response)
+        self.assertTrue(newresponse.body.startswith(b'<!DOCTYPE'))
+        self.assertNotIn('Content-Encoding', newresponse.headers)
 
     def test_process_response_gzip_app_octetstream_contenttype(self):
         response = self._getresponse('gzip')
@@ -183,9 +184,9 @@ class HttpCompressionTest(TestCase):
         request = response.request
 
         newresponse = self.mw.process_response(request, response, self.spider)
-        self.assertIs(newresponse, response)
-        self.assertEqual(response.headers['Content-Encoding'], b'gzip')
-        self.assertEqual(response.headers['Content-Type'], b'application/octet-stream')
+        self.assertIsNot(newresponse, response)
+        self.assertTrue(newresponse.body.startswith(b'<!DOCTYPE'))
+        self.assertNotIn('Content-Encoding', newresponse.headers)
 
     def test_process_response_gzip_binary_octetstream_contenttype(self):
         response = self._getresponse('x-gzip')
@@ -193,9 +194,51 @@ class HttpCompressionTest(TestCase):
         request = response.request
 
         newresponse = self.mw.process_response(request, response, self.spider)
-        self.assertIs(newresponse, response)
-        self.assertEqual(response.headers['Content-Encoding'], b'gzip')
-        self.assertEqual(response.headers['Content-Type'], b'binary/octet-stream')
+        self.assertIsNot(newresponse, response)
+        self.assertTrue(newresponse.body.startswith(b'<!DOCTYPE'))
+        self.assertNotIn('Content-Encoding', newresponse.headers)
+
+    def test_process_response_gzipped_gzip_file(self):
+        """Test that a gzip Content-Encoded .gz file is gunzipped
+        only once by the middleware, leaving gunzipping of the file
+        to upper layers.
+        """
+        headers = {
+            'Content-Type': 'application/gzip',
+            'Content-Encoding': 'gzip',
+        }
+        # build a gzipped file (here, a sitemap)
+        f = BytesIO()
+        plainbody = b"""<?xml version="1.0" encoding="UTF-8"?>
+<urlset xmlns="http://www.google.com/schemas/sitemap/0.84">
+  <url>
+    <loc>http://www.example.com/</loc>
+    <lastmod>2009-08-16</lastmod>
+    <changefreq>daily</changefreq>
+    <priority>1</priority>
+  </url>
+  <url>
+    <loc>http://www.example.com/Special-Offers.html</loc>
+    <lastmod>2009-08-16</lastmod>
+    <changefreq>weekly</changefreq>
+    <priority>0.8</priority>
+  </url>
+</urlset>"""
+        gz_file = GzipFile(fileobj=f, mode='wb')
+        gz_file.write(plainbody)
+        gz_file.close()
+
+        # build a gzipped response body containing this gzipped file
+        r = BytesIO()
+        gz_resp = GzipFile(fileobj=r, mode='wb')
+        gz_resp.write(f.getvalue())
+        gz_resp.close()
+
+        response = Response("http;//www.example.com/", headers=headers, body=r.getvalue())
+        request = Request("http://www.example.com/")
+
+        newresponse = self.mw.process_response(request, response, self.spider)
+        self.assertEqual(gunzip(newresponse.body), plainbody)
 
     def test_process_response_head_request_no_decode_required(self):
         response = self._getresponse('gzip')

tests/test_spider.py

@@ -328,6 +328,10 @@ class SitemapSpiderTest(SpiderTest):
         r = Response(url="http://www.example.com/sitemap.xml.gz", body=self.GZBODY)
         self.assertSitemapBody(r, self.BODY)
 
+        # .xml.gz but body decoded by HttpCompression middleware already
+        r = Response(url="http://www.example.com/sitemap.xml.gz", body=self.BODY)
+        self.assertSitemapBody(r, self.BODY)
+
     def test_get_sitemap_urls_from_robotstxt(self):
         robots = b"""# Sitemap files
 Sitemap: http://example.com/sitemap.xml