From 6969041c5f6891a0298d7e68ece762adee1bb222 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 22 Nov 2023 15:52:00 +0100 Subject: [PATCH 01/20] Protect against gzip bombs --- .../downloadermiddlewares/httpcompression.py | 26 +++++++++++----- scrapy/utils/_compression.py | 2 ++ scrapy/utils/gz.py | 14 +++++++-- tests/sample_data/compressed/bomb-gzip.bin | Bin 0 -> 27988 bytes ...st_downloadermiddleware_httpcompression.py | 29 ++++++++++++++++-- 5 files changed, 59 insertions(+), 12 deletions(-) create mode 100644 scrapy/utils/_compression.py create mode 100644 tests/sample_data/compressed/bomb-gzip.bin diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index ead426951..5dd67ea87 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -2,9 +2,10 @@ import io import warnings import zlib -from scrapy.exceptions import NotConfigured +from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Response, TextResponse from scrapy.responsetypes import responsetypes +from scrapy.utils._compression import _DecompressionMaxSizeExceeded from scrapy.utils.deprecate import ScrapyDeprecationWarning from scrapy.utils.gz import gunzip @@ -29,24 +30,26 @@ class HttpCompressionMiddleware: """This middleware allows compressed (gzip, deflate) traffic to be sent/received from web sites""" - def __init__(self, stats=None): - self.stats = stats + def __init__(self, crawler=None): + self.stats = crawler.stats + self._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") @classmethod def from_crawler(cls, crawler): if not crawler.settings.getbool("COMPRESSION_ENABLED"): raise NotConfigured try: - return cls(stats=crawler.stats) + return cls(crawler=crawler) except TypeError: warnings.warn( "HttpCompressionMiddleware subclasses must either modify " - "their '__init__' method to support a 'stats' parameter or " - "reimplement the 'from_crawler' method.", + "their '__init__' method to support a 'crawler' parameter or " + "reimplement their 'from_crawler' method.", ScrapyDeprecationWarning, ) result = cls() result.stats = crawler.stats + result._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") return result def process_request(self, request, spider): @@ -59,7 +62,14 @@ class HttpCompressionMiddleware: content_encoding = response.headers.getlist("Content-Encoding") if content_encoding: encoding = content_encoding.pop() - decoded_body = self._decode(response.body, encoding.lower()) + try: + decoded_body = self._decode(response.body, encoding.lower()) + except _DecompressionMaxSizeExceeded: + raise IgnoreRequest( + f"Ignored response {response} because its body " + f"({len(response.body)}B) exceeded DOWNLOAD_MAXSIZE " + f"({self._max_size}B) during decompression." + ) if self.stats: self.stats.inc_value( "httpcompression/response_bytes", @@ -85,7 +95,7 @@ class HttpCompressionMiddleware: def _decode(self, body, encoding): if encoding == b"gzip" or encoding == b"x-gzip": - body = gunzip(body) + body = gunzip(body, max_size=self._max_size) if encoding == b"deflate": try: diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py new file mode 100644 index 000000000..e726a70f5 --- /dev/null +++ b/scrapy/utils/_compression.py @@ -0,0 +1,2 @@ +class _DecompressionMaxSizeExceeded(ValueError): + pass diff --git a/scrapy/utils/gz.py b/scrapy/utils/gz.py index c7f74030e..cd5059a5c 100644 --- a/scrapy/utils/gz.py +++ b/scrapy/utils/gz.py @@ -5,8 +5,10 @@ from typing import List from scrapy.http import Response +from ._compression import _DecompressionMaxSizeExceeded -def gunzip(data: bytes) -> bytes: + +def gunzip(data: bytes, max_size: int = 0) -> bytes: """Gunzip the given data and return as much data as possible. This is resilient to CRC checksum errors. @@ -14,10 +16,10 @@ def gunzip(data: bytes) -> bytes: f = GzipFile(fileobj=BytesIO(data)) output_list: List[bytes] = [] chunk = b"." + decompressed_size = 0 while chunk: try: chunk = f.read1(8196) - output_list.append(chunk) except (OSError, EOFError, struct.error): # complete only if there is some data, otherwise re-raise # see issue 87 about catching struct.error @@ -25,6 +27,14 @@ def gunzip(data: bytes) -> bytes: if output_list: break raise + decompressed_size += len(chunk) + if max_size and decompressed_size > max_size: + raise _DecompressionMaxSizeExceeded( + f"The number of bytes decompressed so far " + f"({decompressed_size}B) exceed the specified maximum " + f"({max_size}B)." + ) + output_list.append(chunk) return b"".join(output_list) diff --git a/tests/sample_data/compressed/bomb-gzip.bin b/tests/sample_data/compressed/bomb-gzip.bin new file mode 100644 index 0000000000000000000000000000000000000000..64aa0c3696cc1c6d86d218c70635d88f2035ec9e GIT binary patch literal 27988 zcmeIyElY!86b9f&oiK|G(Y#;~27Xl0-yq1UE0YN_<|{r6TM$G+ga1HfWkC=@i}}T- zAQQ0;tA;J=f*|@M2A1oDJDzYj_mw}*X4m_rN*LQrYP)-t7`Kz1`EpV#FVq|L(0ja} zI9SSs+qBrtti6t3Pxsob#`n?W)Wc%`Y$l!UY&@@AZN0t3&;4p=`TZgaH}D5)fC3Vd zkc1>8Aqh!HLK2dYgd`*(2}wvo5|WUFBqSjTNk~Exl8}TXBq0e&NJ0{lkc1>8Aqh!H zLK2dYgd`*(2}wvo5|WUFBqSjTNk~Exl8}TXBq0e&NJ0{lkc1>8Aqh!HLK2d2h!PI& z=1wx8 S;eSfl{TO{ZZ^qTjoA3)j<4WiN literal 0 HcmV?d00001 diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index 9dad056de..f834e78f5 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -10,7 +10,7 @@ from scrapy.downloadermiddlewares.httpcompression import ( ACCEPTED_ENCODINGS, HttpCompressionMiddleware, ) -from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning +from scrapy.exceptions import IgnoreRequest, NotConfigured, ScrapyDeprecationWarning from scrapy.http import HtmlResponse, Request, Response from scrapy.responsetypes import responsetypes from scrapy.spiders import Spider @@ -35,12 +35,24 @@ FORMAT = { "html-zstd-streaming-no-content-size.bin", "zstd", ), + **{ + f"bomb-{format_id}": (f"bomb-{format_id}.bin", format_id) + for format_id in ( + # "br", + "gzip", # 27 988 → 11 511 612 + # "deflate", + # "zstd", + ) + }, } class HttpCompressionTest(TestCase): def setUp(self): - self.crawler = get_crawler(Spider) + settings = { + "DOWNLOAD_MAXSIZE": 10_000_000, # For compression bomb tests. + } + self.crawler = get_crawler(Spider, settings_dict=settings) self.spider = self.crawler._create_spider("scrapytest.org") self.mw = HttpCompressionMiddleware.from_crawler(self.crawler) self.crawler.stats.open_spider(self.spider) @@ -373,6 +385,19 @@ class HttpCompressionTest(TestCase): self.assertStatsEqual("httpcompression/response_count", None) self.assertStatsEqual("httpcompression/response_bytes", None) + def _test_compression_bomb(self, compression_id): + response = self._getresponse(f"bomb-{compression_id}") + self.assertRaises( + IgnoreRequest, + self.mw.process_response, + response.request, + response, + self.spider, + ) + + def test_compression_bomb_gzip(self): + self._test_compression_bomb("gzip") + class HttpCompressionSubclassTest(TestCase): def test_init_missing_stats(self): From 0bf29a7b1b9b6a641c486780b7b0fa455577bf39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 22 Nov 2023 16:10:50 +0100 Subject: [PATCH 02/20] Update test expectations --- scrapy/downloadermiddlewares/httpcompression.py | 4 +++- .../test_downloadermiddleware_httpcompression.py | 16 ++-------------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 5dd67ea87..0fec05a14 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -30,7 +30,9 @@ class HttpCompressionMiddleware: """This middleware allows compressed (gzip, deflate) traffic to be sent/received from web sites""" - def __init__(self, crawler=None): + def __init__(self, *, crawler=None): + if not crawler: + return self.stats = crawler.stats self._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index f834e78f5..f5dedd28d 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -127,18 +127,6 @@ class HttpCompressionTest(TestCase): self.assertStatsEqual("httpcompression/response_count", 1) self.assertStatsEqual("httpcompression/response_bytes", 74837) - def test_process_response_gzip_no_stats(self): - mw = HttpCompressionMiddleware() - response = self._getresponse("gzip") - request = response.request - - self.assertEqual(response.headers["Content-Encoding"], b"gzip") - newresponse = mw.process_response(request, response, self.spider) - self.assertEqual(mw.stats, None) - assert newresponse is not response - assert newresponse.body.startswith(b" Date: Wed, 22 Nov 2023 17:12:43 +0100 Subject: [PATCH 03/20] Protect against deflate bombs --- .../downloadermiddlewares/httpcompression.py | 20 +++------ scrapy/utils/_compression.py | 39 ++++++++++++++++++ scrapy/utils/gz.py | 2 +- tests/sample_data/compressed/bomb-deflate.bin | Bin 0 -> 27968 bytes ...st_downloadermiddleware_httpcompression.py | 5 ++- 5 files changed, 49 insertions(+), 17 deletions(-) create mode 100644 tests/sample_data/compressed/bomb-deflate.bin diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 0fec05a14..8cec87c47 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -1,11 +1,10 @@ import io import warnings -import zlib from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Response, TextResponse from scrapy.responsetypes import responsetypes -from scrapy.utils._compression import _DecompressionMaxSizeExceeded +from scrapy.utils._compression import _DecompressionMaxSizeExceeded, _inflate from scrapy.utils.deprecate import ScrapyDeprecationWarning from scrapy.utils.gz import gunzip @@ -97,23 +96,14 @@ class HttpCompressionMiddleware: def _decode(self, body, encoding): if encoding == b"gzip" or encoding == b"x-gzip": - body = gunzip(body, max_size=self._max_size) - + return gunzip(body, max_size=self._max_size) if encoding == b"deflate": - try: - body = zlib.decompress(body) - except zlib.error: - # ugly hack to work with raw deflate content that may - # be sent by microsoft servers. For more information, see: - # http://carsten.codimi.de/gzip.yaws/ - # http://www.port80software.com/200ok/archive/2005/10/31/868.aspx - # http://www.gzip.org/zlib/zlib_faq.html#faq38 - body = zlib.decompress(body, -15) + return _inflate(body, max_size=self._max_size) if encoding == b"br" and b"br" in ACCEPTED_ENCODINGS: - body = brotli.decompress(body) + return brotli.decompress(body) if encoding == b"zstd" and b"zstd" in ACCEPTED_ENCODINGS: # Using its streaming API since its simple API could handle only cases # where there is content size data embedded in the frame reader = zstandard.ZstdDecompressor().stream_reader(io.BytesIO(body)) - body = reader.read() + return reader.read() return body diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py index e726a70f5..34bf2e4f7 100644 --- a/scrapy/utils/_compression.py +++ b/scrapy/utils/_compression.py @@ -1,2 +1,41 @@ +import zlib +from io import BytesIO +from typing import List + + class _DecompressionMaxSizeExceeded(ValueError): pass + + +def _inflate(data: bytes, *, max_size: int = 0) -> bytes: + decompressor = zlib.decompressobj() + raw_decompressor = zlib.decompressobj(wbits=-15) + input_stream = BytesIO(data) + output_list: List[bytes] = [] + output_chunk = b"." + decompressed_size = 0 + CHUNK_SIZE = 8196 + while output_chunk: + input_chunk = input_stream.read(CHUNK_SIZE) + try: + output_chunk = decompressor.decompress(input_chunk) + except zlib.error: + if decompressor != raw_decompressor: + # ugly hack to work with raw deflate content that may + # be sent by microsoft servers. For more information, see: + # http://carsten.codimi.de/gzip.yaws/ + # http://www.port80software.com/200ok/archive/2005/10/31/868.aspx + # http://www.gzip.org/zlib/zlib_faq.html#faq38 + decompressor = raw_decompressor + output_chunk = decompressor.decompress(input_chunk) + else: + raise + decompressed_size += len(output_chunk) + if max_size and decompressed_size > max_size: + raise _DecompressionMaxSizeExceeded( + f"The number of bytes decompressed so far " + f"({decompressed_size}B) exceed the specified maximum " + f"({max_size}B)." + ) + output_list.append(output_chunk) + return b"".join(output_list) diff --git a/scrapy/utils/gz.py b/scrapy/utils/gz.py index cd5059a5c..548134721 100644 --- a/scrapy/utils/gz.py +++ b/scrapy/utils/gz.py @@ -8,7 +8,7 @@ from scrapy.http import Response from ._compression import _DecompressionMaxSizeExceeded -def gunzip(data: bytes, max_size: int = 0) -> bytes: +def gunzip(data: bytes, *, max_size: int = 0) -> bytes: """Gunzip the given data and return as much data as possible. This is resilient to CRC checksum errors. diff --git a/tests/sample_data/compressed/bomb-deflate.bin b/tests/sample_data/compressed/bomb-deflate.bin new file mode 100644 index 0000000000000000000000000000000000000000..3598aca0777ec2511a721e9655cabb8c21c658bd GIT binary patch literal 27968 zcmeI&u}VS#6b9f+=-?6}`2-Gb=^8GEdrPzhZ7q%$M35jzaB^}JadC@=dVsh@OD>AI zMF>rSC{CdeWcdhg3f~#dd^nu*O@FmB>%Sy!^U2^bI{%2BjpGkTvtGCQb9b0}%gx*A zC^NVm7VfVnqwxEtJUIF4gqj_=18;x=5|WUFBqSjTNk~Exl8}TXBq0e&NJ0{lkc1>8 zAqh!HLK2dYgd`*(2}wvo5|VI-C0ssb8?oTOioa2%K7C$JY75N{+<`Yh0SQS+LK2dY zgd`*(2}wvo5|WUFBqSjTNk~Exl8}TXBq0e&NJ0{lkc1>8Aqh#iPZGYjN(Y-T;OY9R z_8O#jIJamt;d0?};d0?}5|WUFBqSjTNk~Exl8}TXBq0e&NJ0{lkc1>8Aqh#im4waX I@bhBz2V-bE-2eap literal 0 HcmV?d00001 diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index f5dedd28d..3af8202cc 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -39,8 +39,8 @@ FORMAT = { f"bomb-{format_id}": (f"bomb-{format_id}.bin", format_id) for format_id in ( # "br", + "deflate", # 27 968 → 11 511 612 "gzip", # 27 988 → 11 511 612 - # "deflate", # "zstd", ) }, @@ -383,6 +383,9 @@ class HttpCompressionTest(TestCase): self.spider, ) + def test_compression_bomb_deflate(self): + self._test_compression_bomb("deflate") + def test_compression_bomb_gzip(self): self._test_compression_bomb("gzip") From fba167c5e1f356bcc452e95e92199f1a15135c60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 22 Nov 2023 17:32:09 +0100 Subject: [PATCH 04/20] Protect against brotli bombs --- .../downloadermiddlewares/httpcompression.py | 14 +++++----- scrapy/utils/_compression.py | 26 +++++++++++++++++++ tests/sample_data/compressed/bomb-br.bin | 2 ++ ...st_downloadermiddleware_httpcompression.py | 9 ++++++- 4 files changed, 43 insertions(+), 8 deletions(-) create mode 100644 tests/sample_data/compressed/bomb-br.bin diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 8cec87c47..91748e57e 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -4,25 +4,25 @@ import warnings from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Response, TextResponse from scrapy.responsetypes import responsetypes -from scrapy.utils._compression import _DecompressionMaxSizeExceeded, _inflate +from scrapy.utils._compression import _DecompressionMaxSizeExceeded, _inflate, _unbrotli from scrapy.utils.deprecate import ScrapyDeprecationWarning from scrapy.utils.gz import gunzip ACCEPTED_ENCODINGS = [b"gzip", b"deflate"] try: - import brotli - - ACCEPTED_ENCODINGS.append(b"br") + import brotli # noqa: F401 except ImportError: pass +else: + ACCEPTED_ENCODINGS.append(b"br") try: import zstandard - - ACCEPTED_ENCODINGS.append(b"zstd") except ImportError: pass +else: + ACCEPTED_ENCODINGS.append(b"zstd") class HttpCompressionMiddleware: @@ -100,7 +100,7 @@ class HttpCompressionMiddleware: if encoding == b"deflate": return _inflate(body, max_size=self._max_size) if encoding == b"br" and b"br" in ACCEPTED_ENCODINGS: - return brotli.decompress(body) + return _unbrotli(body, max_size=self._max_size) if encoding == b"zstd" and b"zstd" in ACCEPTED_ENCODINGS: # Using its streaming API since its simple API could handle only cases # where there is content size data embedded in the frame diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py index 34bf2e4f7..9a32ce4f0 100644 --- a/scrapy/utils/_compression.py +++ b/scrapy/utils/_compression.py @@ -2,6 +2,11 @@ import zlib from io import BytesIO from typing import List +try: + import brotli +except ImportError: + pass + class _DecompressionMaxSizeExceeded(ValueError): pass @@ -39,3 +44,24 @@ def _inflate(data: bytes, *, max_size: int = 0) -> bytes: ) output_list.append(output_chunk) return b"".join(output_list) + + +def _unbrotli(data: bytes, *, max_size: int = 0) -> bytes: + decompressor = brotli.Decompressor() + input_stream = BytesIO(data) + output_list: List[bytes] = [] + output_chunk = b"." + decompressed_size = 0 + CHUNK_SIZE = 8196 + while output_chunk: + input_chunk = input_stream.read(CHUNK_SIZE) + output_chunk = decompressor.process(input_chunk) + decompressed_size += len(output_chunk) + if max_size and decompressed_size > max_size: + raise _DecompressionMaxSizeExceeded( + f"The number of bytes decompressed so far " + f"({decompressed_size}B) exceed the specified maximum " + f"({max_size}B)." + ) + output_list.append(output_chunk) + return b"".join(output_list) diff --git a/tests/sample_data/compressed/bomb-br.bin b/tests/sample_data/compressed/bomb-br.bin new file mode 100644 index 000000000..50059866f --- /dev/null +++ b/tests/sample_data/compressed/bomb-br.bin @@ -0,0 +1,2 @@ +Ÿ;§¯ø”nުVp SmoY2·å +ï(ä)-Ğ´=_o \ No newline at end of file diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index 3af8202cc..8858916bc 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -38,7 +38,7 @@ FORMAT = { **{ f"bomb-{format_id}": (f"bomb-{format_id}.bin", format_id) for format_id in ( - # "br", + "br", # 34 → 11 511 612 "deflate", # 27 968 → 11 511 612 "gzip", # 27 988 → 11 511 612 # "zstd", @@ -383,6 +383,13 @@ class HttpCompressionTest(TestCase): self.spider, ) + def test_compression_bomb_br(self): + try: + import brotli # noqa: F401 + except ImportError: + raise SkipTest("no brotli") + self._test_compression_bomb("br") + def test_compression_bomb_deflate(self): self._test_compression_bomb("deflate") From 9cc870387745f45f744ceef6ed226eefcce0e066 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 22 Nov 2023 17:53:00 +0100 Subject: [PATCH 05/20] Protect against zstandard bombs --- .../downloadermiddlewares/httpcompression.py | 15 ++++++----- scrapy/utils/_compression.py | 25 ++++++++++++++++++ tests/sample_data/compressed/bomb-zstd.bin | Bin 0 -> 1096 bytes ...st_downloadermiddleware_httpcompression.py | 5 +++- 4 files changed, 37 insertions(+), 8 deletions(-) create mode 100644 tests/sample_data/compressed/bomb-zstd.bin diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 91748e57e..8ee1d95a6 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -1,10 +1,14 @@ -import io import warnings from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Response, TextResponse from scrapy.responsetypes import responsetypes -from scrapy.utils._compression import _DecompressionMaxSizeExceeded, _inflate, _unbrotli +from scrapy.utils._compression import ( + _DecompressionMaxSizeExceeded, + _inflate, + _unbrotli, + _unzstd, +) from scrapy.utils.deprecate import ScrapyDeprecationWarning from scrapy.utils.gz import gunzip @@ -18,7 +22,7 @@ else: ACCEPTED_ENCODINGS.append(b"br") try: - import zstandard + import zstandard # noqa: F401 except ImportError: pass else: @@ -102,8 +106,5 @@ class HttpCompressionMiddleware: if encoding == b"br" and b"br" in ACCEPTED_ENCODINGS: return _unbrotli(body, max_size=self._max_size) if encoding == b"zstd" and b"zstd" in ACCEPTED_ENCODINGS: - # Using its streaming API since its simple API could handle only cases - # where there is content size data embedded in the frame - reader = zstandard.ZstdDecompressor().stream_reader(io.BytesIO(body)) - return reader.read() + return _unzstd(body, max_size=self._max_size) return body diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py index 9a32ce4f0..93aa254b2 100644 --- a/scrapy/utils/_compression.py +++ b/scrapy/utils/_compression.py @@ -7,6 +7,11 @@ try: except ImportError: pass +try: + import zstandard +except ImportError: + pass + class _DecompressionMaxSizeExceeded(ValueError): pass @@ -65,3 +70,23 @@ def _unbrotli(data: bytes, *, max_size: int = 0) -> bytes: ) output_list.append(output_chunk) return b"".join(output_list) + + +def _unzstd(data: bytes, *, max_size: int = 0) -> bytes: + decompressor = zstandard.ZstdDecompressor() + stream_reader = decompressor.stream_reader(BytesIO(data)) + output_list: List[bytes] = [] + output_chunk = b"." + decompressed_size = 0 + CHUNK_SIZE = 8196 + while output_chunk: + output_chunk = stream_reader.read(CHUNK_SIZE) + decompressed_size += len(output_chunk) + if max_size and decompressed_size > max_size: + raise _DecompressionMaxSizeExceeded( + f"The number of bytes decompressed so far " + f"({decompressed_size}B) exceed the specified maximum " + f"({max_size}B)." + ) + output_list.append(output_chunk) + return b"".join(output_list) diff --git a/tests/sample_data/compressed/bomb-zstd.bin b/tests/sample_data/compressed/bomb-zstd.bin new file mode 100644 index 0000000000000000000000000000000000000000..4b0efa8a41c88a38dffe7cea9e4a6726bdb137c4 GIT binary patch literal 1096 zcmdPcs{gko!e;q;1{Fqz2O$}m#R@=_sF0kWTTql*T%4Jor;wDNo219Z$k6h?-fpfB z0|SQwBg3EnmI6#5b|Mlx7l~br#Lh!vBdZBP5+5~lG&~1uT5@4vU|?kU`+vT_!f29* VWPM*?)(2)~i{-##pxebr9RRD(6-595 literal 0 HcmV?d00001 diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index 8858916bc..7babd1318 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -41,7 +41,7 @@ FORMAT = { "br", # 34 → 11 511 612 "deflate", # 27 968 → 11 511 612 "gzip", # 27 988 → 11 511 612 - # "zstd", + "zstd", # 1 096 → 11 511 612 ) }, } @@ -396,6 +396,9 @@ class HttpCompressionTest(TestCase): def test_compression_bomb_gzip(self): self._test_compression_bomb("gzip") + def test_compression_bomb_zstd(self): + self._test_compression_bomb("zstd") + class HttpCompressionSubclassTest(TestCase): def test_init_missing_stats(self): From 3fda2fe103dafa8d4d48b21c63b2321ccec9c378 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 22 Nov 2023 18:34:37 +0100 Subject: [PATCH 06/20] Protect against gzip bomb sitemaps --- scrapy/spiders/sitemap.py | 8 +++++++- tests/test_spider.py | 15 +++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/scrapy/spiders/sitemap.py b/scrapy/spiders/sitemap.py index aaf75a519..cc8b13cc3 100644 --- a/scrapy/spiders/sitemap.py +++ b/scrapy/spiders/sitemap.py @@ -3,6 +3,7 @@ import re from scrapy.http import Request, XmlResponse from scrapy.spiders import Spider +from scrapy.utils._compression import _DecompressionMaxSizeExceeded from scrapy.utils.gz import gunzip, gzip_magic_number from scrapy.utils.sitemap import Sitemap, sitemap_urls_from_robots @@ -71,7 +72,12 @@ class SitemapSpider(Spider): if isinstance(response, XmlResponse): return response.body if gzip_magic_number(response): - return gunzip(response.body) + try: + return gunzip( + response.body, max_size=self.settings.getint("DOWNLOAD_MAXSIZE") + ) + except _DecompressionMaxSizeExceeded: + return None # actual gzipped sitemap files are decompressed above ; # if we are here (response body is not gzipped) # and have a response for .xml.gz, diff --git a/tests/test_spider.py b/tests/test_spider.py index 00da3d485..875ff5454 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -2,6 +2,7 @@ import gzip import inspect import warnings from io import BytesIO +from pathlib import Path from typing import Any from unittest import mock @@ -25,7 +26,7 @@ from scrapy.spiders import ( ) from scrapy.spiders.init import InitSpider from scrapy.utils.test import get_crawler -from tests import get_testdata +from tests import get_testdata, tests_datadir class SpiderTest(unittest.TestCase): @@ -489,7 +490,8 @@ class SitemapSpiderTest(SpiderTest): GZBODY = f.getvalue() def assertSitemapBody(self, response, body): - spider = self.spider_class("example.com") + crawler = get_crawler() + spider = self.spider_class.from_crawler(crawler, "example.com") self.assertEqual(spider._get_sitemap_body(response), body) def test_get_sitemap_body(self): @@ -692,6 +694,15 @@ Sitemap: /sitemap-relative-url.xml ["http://www.example.com/sitemap2.xml"], ) + def test_compression_bomb(self): + settings = {"DOWNLOAD_MAXSIZE": 10_000_000} + crawler = get_crawler(settings_dict=settings) + spider = self.spider_class.from_crawler(crawler, "example.com") + body_path = Path(tests_datadir, "compressed", "bomb-gzip.bin") + body = body_path.read_bytes() + response = Response(url="https://example.com", body=body) + self.assertIsNone(spider._get_sitemap_body(response)) + class DeprecationTest(unittest.TestCase): def test_crawl_spider(self): From e0b66c021ae20cdcc24e4bb02ffae56005d3a073 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 22 Nov 2023 19:03:24 +0100 Subject: [PATCH 07/20] Mind Spider.download_maxsize and Request.meta['download_maxsize'] --- .../downloadermiddlewares/httpcompression.py | 30 ++++-- scrapy/spiders/sitemap.py | 10 +- ...st_downloadermiddleware_httpcompression.py | 99 ++++++++++++++++--- tests/test_spider.py | 35 ++++++- 4 files changed, 143 insertions(+), 31 deletions(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 8ee1d95a6..e6463307e 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -1,5 +1,6 @@ import warnings +from scrapy import signals from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Response, TextResponse from scrapy.responsetypes import responsetypes @@ -38,6 +39,7 @@ class HttpCompressionMiddleware: return self.stats = crawler.stats self._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") + crawler.signals.connect(self.open_spider, signals.spider_opened) @classmethod def from_crawler(cls, crawler): @@ -52,10 +54,15 @@ class HttpCompressionMiddleware: "reimplement their 'from_crawler' method.", ScrapyDeprecationWarning, ) - result = cls() - result.stats = crawler.stats - result._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") - return result + spider = cls() + spider.stats = crawler.stats + spider._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") + crawler.signals.connect(spider.open_spider, signals.spider_opened) + return spider + + def open_spider(self, spider): + if hasattr(spider, "download_maxsize"): + self._max_size = spider.download_maxsize def process_request(self, request, spider): request.headers.setdefault("Accept-Encoding", b", ".join(ACCEPTED_ENCODINGS)) @@ -67,8 +74,11 @@ class HttpCompressionMiddleware: content_encoding = response.headers.getlist("Content-Encoding") if content_encoding: encoding = content_encoding.pop() + max_size = request.meta.get("download_maxsize", self._max_size) try: - decoded_body = self._decode(response.body, encoding.lower()) + decoded_body = self._decode( + response.body, encoding.lower(), max_size + ) except _DecompressionMaxSizeExceeded: raise IgnoreRequest( f"Ignored response {response} because its body " @@ -98,13 +108,13 @@ class HttpCompressionMiddleware: return response - def _decode(self, body, encoding): + def _decode(self, body, encoding, max_size): if encoding == b"gzip" or encoding == b"x-gzip": - return gunzip(body, max_size=self._max_size) + return gunzip(body, max_size=max_size) if encoding == b"deflate": - return _inflate(body, max_size=self._max_size) + return _inflate(body, max_size=max_size) if encoding == b"br" and b"br" in ACCEPTED_ENCODINGS: - return _unbrotli(body, max_size=self._max_size) + return _unbrotli(body, max_size=max_size) if encoding == b"zstd" and b"zstd" in ACCEPTED_ENCODINGS: - return _unzstd(body, max_size=self._max_size) + return _unzstd(body, max_size=max_size) return body diff --git a/scrapy/spiders/sitemap.py b/scrapy/spiders/sitemap.py index cc8b13cc3..3bca3f5c2 100644 --- a/scrapy/spiders/sitemap.py +++ b/scrapy/spiders/sitemap.py @@ -72,10 +72,14 @@ class SitemapSpider(Spider): if isinstance(response, XmlResponse): return response.body if gzip_magic_number(response): + max_size = response.meta.get( + "download_maxsize", + getattr( + self, "download_maxsize", self.settings.getint("DOWNLOAD_MAXSIZE") + ), + ) try: - return gunzip( - response.body, max_size=self.settings.getint("DOWNLOAD_MAXSIZE") - ) + return gunzip(response.body, max_size=max_size) except _DecompressionMaxSizeExceeded: return None # actual gzipped sitemap files are decompressed above ; diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index 7babd1318..6d71ba71e 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -49,10 +49,7 @@ FORMAT = { class HttpCompressionTest(TestCase): def setUp(self): - settings = { - "DOWNLOAD_MAXSIZE": 10_000_000, # For compression bomb tests. - } - self.crawler = get_crawler(Spider, settings_dict=settings) + self.crawler = get_crawler(Spider) self.spider = self.crawler._create_spider("scrapytest.org") self.mw = HttpCompressionMiddleware.from_crawler(self.crawler) self.crawler.stats.open_spider(self.spider) @@ -373,31 +370,103 @@ class HttpCompressionTest(TestCase): self.assertStatsEqual("httpcompression/response_count", None) self.assertStatsEqual("httpcompression/response_bytes", None) - def _test_compression_bomb(self, compression_id): + def _test_compression_bomb_setting(self, compression_id): + settings = {"DOWNLOAD_MAXSIZE": 10_000_000} + crawler = get_crawler(Spider, settings_dict=settings) + spider = crawler._create_spider("scrapytest.org") + mw = HttpCompressionMiddleware.from_crawler(crawler) + mw.open_spider(spider) + response = self._getresponse(f"bomb-{compression_id}") self.assertRaises( IgnoreRequest, - self.mw.process_response, + mw.process_response, response.request, response, - self.spider, + spider, ) - def test_compression_bomb_br(self): + def test_compression_bomb_setting_br(self): try: import brotli # noqa: F401 except ImportError: raise SkipTest("no brotli") - self._test_compression_bomb("br") + self._test_compression_bomb_setting("br") - def test_compression_bomb_deflate(self): - self._test_compression_bomb("deflate") + def test_compression_bomb_setting_deflate(self): + self._test_compression_bomb_setting("deflate") - def test_compression_bomb_gzip(self): - self._test_compression_bomb("gzip") + def test_compression_bomb_setting_gzip(self): + self._test_compression_bomb_setting("gzip") - def test_compression_bomb_zstd(self): - self._test_compression_bomb("zstd") + def test_compression_bomb_setting_zstd(self): + self._test_compression_bomb_setting("zstd") + + def _test_compression_bomb_spider_attr(self, compression_id): + class DownloadMaxSizeSpider(Spider): + download_maxsize = 10_000_000 + + crawler = get_crawler(DownloadMaxSizeSpider) + spider = crawler._create_spider("scrapytest.org") + mw = HttpCompressionMiddleware.from_crawler(crawler) + mw.open_spider(spider) + + response = self._getresponse(f"bomb-{compression_id}") + self.assertRaises( + IgnoreRequest, + mw.process_response, + response.request, + response, + spider, + ) + + def test_compression_bomb_spider_attr_br(self): + try: + import brotli # noqa: F401 + except ImportError: + raise SkipTest("no brotli") + self._test_compression_bomb_spider_attr("br") + + def test_compression_bomb_spider_attr_deflate(self): + self._test_compression_bomb_spider_attr("deflate") + + def test_compression_bomb_spider_attr_gzip(self): + self._test_compression_bomb_spider_attr("gzip") + + def test_compression_bomb_spider_attr_zstd(self): + self._test_compression_bomb_spider_attr("zstd") + + def _test_compression_bomb_request_meta(self, compression_id): + crawler = get_crawler(Spider) + spider = crawler._create_spider("scrapytest.org") + mw = HttpCompressionMiddleware.from_crawler(crawler) + mw.open_spider(spider) + + response = self._getresponse(f"bomb-{compression_id}") + response.meta["download_maxsize"] = 10_000_000 + self.assertRaises( + IgnoreRequest, + mw.process_response, + response.request, + response, + spider, + ) + + def test_compression_bomb_request_meta_br(self): + try: + import brotli # noqa: F401 + except ImportError: + raise SkipTest("no brotli") + self._test_compression_bomb_request_meta("br") + + def test_compression_bomb_request_meta_deflate(self): + self._test_compression_bomb_request_meta("deflate") + + def test_compression_bomb_request_meta_gzip(self): + self._test_compression_bomb_request_meta("gzip") + + def test_compression_bomb_request_meta_zstd(self): + self._test_compression_bomb_request_meta("zstd") class HttpCompressionSubclassTest(TestCase): diff --git a/tests/test_spider.py b/tests/test_spider.py index 875ff5454..e8480ceb4 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -509,6 +509,7 @@ class SitemapSpiderTest(SpiderTest): url="http://www.example.com/sitemap", body=self.GZBODY, headers={"content-type": "application/gzip"}, + request=Request("http://www.example.com/sitemap"), ) self.assertSitemapBody(r, self.BODY) @@ -517,7 +518,11 @@ class SitemapSpiderTest(SpiderTest): self.assertSitemapBody(r, self.BODY) def test_get_sitemap_body_xml_url_compressed(self): - r = Response(url="http://www.example.com/sitemap.xml.gz", body=self.GZBODY) + r = Response( + url="http://www.example.com/sitemap.xml.gz", + body=self.GZBODY, + request=Request("http://www.example.com/sitemap"), + ) self.assertSitemapBody(r, self.BODY) # .xml.gz but body decoded by HttpCompression middleware already @@ -694,13 +699,37 @@ Sitemap: /sitemap-relative-url.xml ["http://www.example.com/sitemap2.xml"], ) - def test_compression_bomb(self): + def test_compression_bomb_setting(self): settings = {"DOWNLOAD_MAXSIZE": 10_000_000} crawler = get_crawler(settings_dict=settings) spider = self.spider_class.from_crawler(crawler, "example.com") body_path = Path(tests_datadir, "compressed", "bomb-gzip.bin") body = body_path.read_bytes() - response = Response(url="https://example.com", body=body) + request = Request(url="https://example.com") + response = Response(url="https://example.com", body=body, request=request) + self.assertIsNone(spider._get_sitemap_body(response)) + + def test_compression_bomb_spider_attr(self): + class DownloadMaxSizeSpider(self.spider_class): + download_maxsize = 10_000_000 + + crawler = get_crawler() + spider = DownloadMaxSizeSpider.from_crawler(crawler, "example.com") + body_path = Path(tests_datadir, "compressed", "bomb-gzip.bin") + body = body_path.read_bytes() + request = Request(url="https://example.com") + response = Response(url="https://example.com", body=body, request=request) + self.assertIsNone(spider._get_sitemap_body(response)) + + def test_compression_bomb_request_meta(self): + crawler = get_crawler() + spider = self.spider_class.from_crawler(crawler, "example.com") + body_path = Path(tests_datadir, "compressed", "bomb-gzip.bin") + body = body_path.read_bytes() + request = Request( + url="https://example.com", meta={"download_maxsize": 10_000_000} + ) + response = Response(url="https://example.com", body=body, request=request) self.assertIsNone(spider._get_sitemap_body(response)) From 1087bb7b2eab28543bf9ba13149adb7acd4a3675 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 23 Nov 2023 09:11:14 +0100 Subject: [PATCH 08/20] Update the docs --- docs/topics/request-response.rst | 1 + docs/topics/settings.rst | 36 +++++++++++++++++--------------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index adf3d0f4a..2d1227cf8 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -702,6 +702,7 @@ Those are: * :reqmeta:`download_fail_on_dataloss` * :reqmeta:`download_latency` * :reqmeta:`download_maxsize` +* :reqmeta:`download_warnsize` * :reqmeta:`download_timeout` * ``ftp_password`` (See :setting:`FTP_PASSWORD` for more info) * ``ftp_user`` (See :setting:`FTP_USER` for more info) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 7cdfb8768..eb24b834a 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -873,40 +873,42 @@ The amount of time (in secs) that the downloader will wait before timing out. Request.meta key. .. setting:: DOWNLOAD_MAXSIZE +.. reqmeta:: download_maxsize DOWNLOAD_MAXSIZE ---------------- -Default: ``1073741824`` (1024MB) +Default: ``1073741824`` (1 GiB) -The maximum response size (in bytes) that downloader will download. +The maximum response body size (in bytes) allowed. Bigger responses are +aborted and ignored. -If you want to disable it set to 0. +This applies both before and after compression. If decompressing a response +body would exceed this limit, decompression is aborted and the response is +ignored. -.. reqmeta:: download_maxsize +Use ``0`` to disable this limit. -.. note:: - - This size can be set per spider using :attr:`download_maxsize` - spider attribute and per-request using :reqmeta:`download_maxsize` - Request.meta key. +This limit can be set per spider using the :attr:`download_maxsize` spider +attribute and per request using the :reqmeta:`download_maxsize` Request.meta +key. .. setting:: DOWNLOAD_WARNSIZE +.. reqmeta:: download_warnsize DOWNLOAD_WARNSIZE ----------------- -Default: ``33554432`` (32MB) +Default: ``33554432`` (32 MiB) -The response size (in bytes) that downloader will start to warn. +If the size of a response exceeds this value, before or after compression, a +warning will be logged about it. -If you want to disable it set to 0. +Use ``0`` to disable this limit. -.. note:: - - This size can be set per spider using :attr:`download_warnsize` - spider attribute and per-request using :reqmeta:`download_warnsize` - Request.meta key. +This limit can be set per spider using the :attr:`download_warnsize` spider +attribute and per request using the :reqmeta:`download_warnsize` Request.meta +key. .. setting:: DOWNLOAD_FAIL_ON_DATALOSS From 03d9866518ab43844ba0309394529240f4cf115e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 23 Nov 2023 10:26:47 +0100 Subject: [PATCH 09/20] Also use DOWNLOAD_WARNSIZE for decompressions --- .../downloadermiddlewares/httpcompression.py | 18 ++- scrapy/spiders/sitemap.py | 35 ++++- scrapy/utils/_compression.py | 12 +- scrapy/utils/gz.py | 4 +- ...st_downloadermiddleware_httpcompression.py | 130 ++++++++++++++++++ tests/test_spider.py | 78 +++++++++++ 6 files changed, 260 insertions(+), 17 deletions(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index e6463307e..95bc1849d 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -1,4 +1,5 @@ import warnings +from logging import getLogger from scrapy import signals from scrapy.exceptions import IgnoreRequest, NotConfigured @@ -13,6 +14,8 @@ from scrapy.utils._compression import ( from scrapy.utils.deprecate import ScrapyDeprecationWarning from scrapy.utils.gz import gunzip +logger = getLogger(__name__) + ACCEPTED_ENCODINGS = [b"gzip", b"deflate"] try: @@ -39,6 +42,7 @@ class HttpCompressionMiddleware: return self.stats = crawler.stats self._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") + self._warn_size = crawler.settings.getint("DOWNLOAD_WARNSIZE") crawler.signals.connect(self.open_spider, signals.spider_opened) @classmethod @@ -57,12 +61,15 @@ class HttpCompressionMiddleware: spider = cls() spider.stats = crawler.stats spider._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") + spider._warn_size = crawler.settings.getint("DOWNLOAD_WARNSIZE") crawler.signals.connect(spider.open_spider, signals.spider_opened) return spider def open_spider(self, spider): if hasattr(spider, "download_maxsize"): self._max_size = spider.download_maxsize + if hasattr(spider, "download_warnsize"): + self._warn_size = spider.download_warnsize def process_request(self, request, spider): request.headers.setdefault("Accept-Encoding", b", ".join(ACCEPTED_ENCODINGS)) @@ -75,6 +82,7 @@ class HttpCompressionMiddleware: if content_encoding: encoding = content_encoding.pop() max_size = request.meta.get("download_maxsize", self._max_size) + warn_size = request.meta.get("download_warnsize", self._warn_size) try: decoded_body = self._decode( response.body, encoding.lower(), max_size @@ -82,8 +90,14 @@ class HttpCompressionMiddleware: except _DecompressionMaxSizeExceeded: raise IgnoreRequest( f"Ignored response {response} because its body " - f"({len(response.body)}B) exceeded DOWNLOAD_MAXSIZE " - f"({self._max_size}B) during decompression." + f"({len(response.body)} B) exceeded DOWNLOAD_MAXSIZE " + f"({self._max_size} B) during decompression." + ) + if len(response.body) < warn_size and len(decoded_body) >= warn_size: + logger.warning( + f"{response} body size after decompression " + f"({len(decoded_body)} B) is larger than the " + f"download warning size ({warn_size} B)." ) if self.stats: self.stats.inc_value( diff --git a/scrapy/spiders/sitemap.py b/scrapy/spiders/sitemap.py index 3bca3f5c2..0574f0ccb 100644 --- a/scrapy/spiders/sitemap.py +++ b/scrapy/spiders/sitemap.py @@ -1,5 +1,6 @@ import logging import re +from typing import TYPE_CHECKING, Any from scrapy.http import Request, XmlResponse from scrapy.spiders import Spider @@ -7,6 +8,12 @@ from scrapy.utils._compression import _DecompressionMaxSizeExceeded from scrapy.utils.gz import gunzip, gzip_magic_number from scrapy.utils.sitemap import Sitemap, sitemap_urls_from_robots +if TYPE_CHECKING: + # typing.Self requires Python 3.11 + from typing_extensions import Self + + from scrapy.crawler import Crawler + logger = logging.getLogger(__name__) @@ -16,6 +23,17 @@ class SitemapSpider(Spider): sitemap_follow = [""] sitemap_alternate_links = False + @classmethod + def from_crawler(cls, crawler: "Crawler", *args: Any, **kwargs: Any) -> "Self": + spider = super().from_crawler(crawler, *args, **kwargs) + spider._max_size = getattr( + spider, "download_maxsize", spider.settings.getint("DOWNLOAD_MAXSIZE") + ) + spider._warn_size = getattr( + spider, "download_warnsize", spider.settings.getint("DOWNLOAD_WARNSIZE") + ) + return spider + def __init__(self, *a, **kw): super().__init__(*a, **kw) self._cbs = [] @@ -72,16 +90,19 @@ class SitemapSpider(Spider): if isinstance(response, XmlResponse): return response.body if gzip_magic_number(response): - max_size = response.meta.get( - "download_maxsize", - getattr( - self, "download_maxsize", self.settings.getint("DOWNLOAD_MAXSIZE") - ), - ) + uncompressed_size = len(response.body) + max_size = response.meta.get("download_maxsize", self._max_size) + warn_size = response.meta.get("download_warnsize", self._warn_size) try: - return gunzip(response.body, max_size=max_size) + body = gunzip(response.body, max_size=max_size) except _DecompressionMaxSizeExceeded: return None + if uncompressed_size < warn_size and len(body) >= warn_size: + logger.warning( + f"{response} body size after decompression ({len(body)} B) " + f"is larger than the download warning size ({warn_size} B)." + ) + return body # actual gzipped sitemap files are decompressed above ; # if we are here (response body is not gzipped) # and have a response for .xml.gz, diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py index 93aa254b2..a70f6c275 100644 --- a/scrapy/utils/_compression.py +++ b/scrapy/utils/_compression.py @@ -44,8 +44,8 @@ def _inflate(data: bytes, *, max_size: int = 0) -> bytes: if max_size and decompressed_size > max_size: raise _DecompressionMaxSizeExceeded( f"The number of bytes decompressed so far " - f"({decompressed_size}B) exceed the specified maximum " - f"({max_size}B)." + f"({decompressed_size} B) exceed the specified maximum " + f"({max_size} B)." ) output_list.append(output_chunk) return b"".join(output_list) @@ -65,8 +65,8 @@ def _unbrotli(data: bytes, *, max_size: int = 0) -> bytes: if max_size and decompressed_size > max_size: raise _DecompressionMaxSizeExceeded( f"The number of bytes decompressed so far " - f"({decompressed_size}B) exceed the specified maximum " - f"({max_size}B)." + f"({decompressed_size} B) exceed the specified maximum " + f"({max_size} B)." ) output_list.append(output_chunk) return b"".join(output_list) @@ -85,8 +85,8 @@ def _unzstd(data: bytes, *, max_size: int = 0) -> bytes: if max_size and decompressed_size > max_size: raise _DecompressionMaxSizeExceeded( f"The number of bytes decompressed so far " - f"({decompressed_size}B) exceed the specified maximum " - f"({max_size}B)." + f"({decompressed_size} B) exceed the specified maximum " + f"({max_size} B)." ) output_list.append(output_chunk) return b"".join(output_list) diff --git a/scrapy/utils/gz.py b/scrapy/utils/gz.py index 548134721..e5cf68d62 100644 --- a/scrapy/utils/gz.py +++ b/scrapy/utils/gz.py @@ -31,8 +31,8 @@ def gunzip(data: bytes, *, max_size: int = 0) -> bytes: if max_size and decompressed_size > max_size: raise _DecompressionMaxSizeExceeded( f"The number of bytes decompressed so far " - f"({decompressed_size}B) exceed the specified maximum " - f"({max_size}B)." + f"({decompressed_size} B) exceed the specified maximum " + f"({max_size} B)." ) output_list.append(chunk) return b"".join(output_list) diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index 6d71ba71e..f74fff218 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -1,9 +1,11 @@ from gzip import GzipFile from io import BytesIO +from logging import WARNING from pathlib import Path from unittest import SkipTest, TestCase from warnings import catch_warnings +from testfixtures import LogCapture from w3lib.encoding import resolve_encoding from scrapy.downloadermiddlewares.httpcompression import ( @@ -468,6 +470,134 @@ class HttpCompressionTest(TestCase): def test_compression_bomb_request_meta_zstd(self): self._test_compression_bomb_request_meta("zstd") + def _test_download_warnsize_setting(self, compression_id): + settings = {"DOWNLOAD_WARNSIZE": 10_000_000} + crawler = get_crawler(Spider, settings_dict=settings) + spider = crawler._create_spider("scrapytest.org") + mw = HttpCompressionMiddleware.from_crawler(crawler) + mw.open_spider(spider) + response = self._getresponse(f"bomb-{compression_id}") + + with LogCapture( + "scrapy.downloadermiddlewares.httpcompression", + propagate=False, + level=WARNING, + ) as log: + mw.process_response(response.request, response, spider) + log.check( + ( + "scrapy.downloadermiddlewares.httpcompression", + "WARNING", + ( + "<200 http://scrapytest.org/> body size after " + "decompression (11511612 B) is larger than the download " + "warning size (10000000 B)." + ), + ), + ) + + def test_download_warnsize_setting_br(self): + try: + import brotli # noqa: F401 + except ImportError: + raise SkipTest("no brotli") + self._test_download_warnsize_setting("br") + + def test_download_warnsize_setting_deflate(self): + self._test_download_warnsize_setting("deflate") + + def test_download_warnsize_setting_gzip(self): + self._test_download_warnsize_setting("gzip") + + def test_download_warnsize_setting_zstd(self): + self._test_download_warnsize_setting("zstd") + + def _test_download_warnsize_spider_attr(self, compression_id): + class DownloadWarnSizeSpider(Spider): + download_warnsize = 10_000_000 + + crawler = get_crawler(DownloadWarnSizeSpider) + spider = crawler._create_spider("scrapytest.org") + mw = HttpCompressionMiddleware.from_crawler(crawler) + mw.open_spider(spider) + response = self._getresponse(f"bomb-{compression_id}") + + with LogCapture( + "scrapy.downloadermiddlewares.httpcompression", + propagate=False, + level=WARNING, + ) as log: + mw.process_response(response.request, response, spider) + log.check( + ( + "scrapy.downloadermiddlewares.httpcompression", + "WARNING", + ( + "<200 http://scrapytest.org/> body size after " + "decompression (11511612 B) is larger than the download " + "warning size (10000000 B)." + ), + ), + ) + + def test_download_warnsize_spider_attr_br(self): + try: + import brotli # noqa: F401 + except ImportError: + raise SkipTest("no brotli") + self._test_download_warnsize_spider_attr("br") + + def test_download_warnsize_spider_attr_deflate(self): + self._test_download_warnsize_spider_attr("deflate") + + def test_download_warnsize_spider_attr_gzip(self): + self._test_download_warnsize_spider_attr("gzip") + + def test_download_warnsize_spider_attr_zstd(self): + self._test_download_warnsize_spider_attr("zstd") + + def _test_download_warnsize_request_meta(self, compression_id): + crawler = get_crawler(Spider) + spider = crawler._create_spider("scrapytest.org") + mw = HttpCompressionMiddleware.from_crawler(crawler) + mw.open_spider(spider) + response = self._getresponse(f"bomb-{compression_id}") + response.meta["download_warnsize"] = 10_000_000 + + with LogCapture( + "scrapy.downloadermiddlewares.httpcompression", + propagate=False, + level=WARNING, + ) as log: + mw.process_response(response.request, response, spider) + log.check( + ( + "scrapy.downloadermiddlewares.httpcompression", + "WARNING", + ( + "<200 http://scrapytest.org/> body size after " + "decompression (11511612 B) is larger than the download " + "warning size (10000000 B)." + ), + ), + ) + + def test_download_warnsize_request_meta_br(self): + try: + import brotli # noqa: F401 + except ImportError: + raise SkipTest("no brotli") + self._test_download_warnsize_request_meta("br") + + def test_download_warnsize_request_meta_deflate(self): + self._test_download_warnsize_request_meta("deflate") + + def test_download_warnsize_request_meta_gzip(self): + self._test_download_warnsize_request_meta("gzip") + + def test_download_warnsize_request_meta_zstd(self): + self._test_download_warnsize_request_meta("zstd") + class HttpCompressionSubclassTest(TestCase): def test_init_missing_stats(self): diff --git a/tests/test_spider.py b/tests/test_spider.py index e8480ceb4..3f595cc93 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -2,6 +2,7 @@ import gzip import inspect import warnings from io import BytesIO +from logging import WARNING from pathlib import Path from typing import Any from unittest import mock @@ -732,6 +733,83 @@ Sitemap: /sitemap-relative-url.xml response = Response(url="https://example.com", body=body, request=request) self.assertIsNone(spider._get_sitemap_body(response)) + def test_download_warnsize_setting(self): + settings = {"DOWNLOAD_WARNSIZE": 10_000_000} + crawler = get_crawler(settings_dict=settings) + spider = self.spider_class.from_crawler(crawler, "example.com") + body_path = Path(tests_datadir, "compressed", "bomb-gzip.bin") + body = body_path.read_bytes() + request = Request(url="https://example.com") + response = Response(url="https://example.com", body=body, request=request) + with LogCapture( + "scrapy.spiders.sitemap", propagate=False, level=WARNING + ) as log: + spider._get_sitemap_body(response) + log.check( + ( + "scrapy.spiders.sitemap", + "WARNING", + ( + "<200 https://example.com> body size after decompression " + "(11511612 B) is larger than the download warning size " + "(10000000 B)." + ), + ), + ) + + def test_download_warnsize_spider_attr(self): + class DownloadWarnSizeSpider(self.spider_class): + download_warnsize = 10_000_000 + + crawler = get_crawler() + spider = DownloadWarnSizeSpider.from_crawler(crawler, "example.com") + body_path = Path(tests_datadir, "compressed", "bomb-gzip.bin") + body = body_path.read_bytes() + request = Request( + url="https://example.com", meta={"download_warnsize": 10_000_000} + ) + response = Response(url="https://example.com", body=body, request=request) + with LogCapture( + "scrapy.spiders.sitemap", propagate=False, level=WARNING + ) as log: + spider._get_sitemap_body(response) + log.check( + ( + "scrapy.spiders.sitemap", + "WARNING", + ( + "<200 https://example.com> body size after decompression " + "(11511612 B) is larger than the download warning size " + "(10000000 B)." + ), + ), + ) + + def test_download_warnsize_request_meta(self): + crawler = get_crawler() + spider = self.spider_class.from_crawler(crawler, "example.com") + body_path = Path(tests_datadir, "compressed", "bomb-gzip.bin") + body = body_path.read_bytes() + request = Request( + url="https://example.com", meta={"download_warnsize": 10_000_000} + ) + response = Response(url="https://example.com", body=body, request=request) + with LogCapture( + "scrapy.spiders.sitemap", propagate=False, level=WARNING + ) as log: + spider._get_sitemap_body(response) + log.check( + ( + "scrapy.spiders.sitemap", + "WARNING", + ( + "<200 https://example.com> body size after decompression " + "(11511612 B) is larger than the download warning size " + "(10000000 B)." + ), + ), + ) + class DeprecationTest(unittest.TestCase): def test_crawl_spider(self): From b53ed52a22470adbe269f5a7dcc67a0da369eaf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 23 Nov 2023 11:36:45 +0100 Subject: [PATCH 10/20] Update the release notes --- docs/news.rst | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/docs/news.rst b/docs/news.rst index 0c202639e..c4081b99b 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -3,6 +3,20 @@ Release notes ============= +.. _release-2.11.1: + +Scrapy 2.11.1 (unreleased) +-------------------------- + +**Security bug fix:** + +- :setting:`DOWNLOAD_MAXSIZE` and :setting:`DOWNLOAD_WARNSIZE` now also apply + to the decompressed response body. Please, see the `7j7m-v7m3-jqm7 security + advisory`_ for more information. + + .. _7j7m-v7m3-jqm7 security advisory: https://github.com/scrapy/scrapy/security/advisories/GHSA-7j7m-v7m3-jqm7 + + .. _release-2.11.0: Scrapy 2.11.0 (2023-09-18) @@ -2871,6 +2885,17 @@ affect subclasses: (:issue:`3884`) +.. _release-1.8.4: + +Scrapy 1.8.4 (unreleased) +------------------------- + +**Security bug fix:** + +- :setting:`DOWNLOAD_MAXSIZE` and :setting:`DOWNLOAD_WARNSIZE` now also apply + to the decompressed response body. Please, see the `7j7m-v7m3-jqm7 security + advisory`_ for more information. + .. _release-1.8.3: From cf80e5670e8317858b9f60008a5d2d97b4988da0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 23 Nov 2023 12:07:15 +0100 Subject: [PATCH 11/20] Solve linting and typing issues --- scrapy/downloadermiddlewares/httpcompression.py | 2 +- scrapy/spiders/sitemap.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 95bc1849d..6c8b659bd 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -93,7 +93,7 @@ class HttpCompressionMiddleware: f"({len(response.body)} B) exceeded DOWNLOAD_MAXSIZE " f"({self._max_size} B) during decompression." ) - if len(response.body) < warn_size and len(decoded_body) >= warn_size: + if len(response.body) < warn_size <= len(decoded_body): logger.warning( f"{response} body size after decompression " f"({len(decoded_body)} B) is larger than the " diff --git a/scrapy/spiders/sitemap.py b/scrapy/spiders/sitemap.py index 0574f0ccb..386aa6a6e 100644 --- a/scrapy/spiders/sitemap.py +++ b/scrapy/spiders/sitemap.py @@ -22,6 +22,8 @@ class SitemapSpider(Spider): sitemap_rules = [("", "parse")] sitemap_follow = [""] sitemap_alternate_links = False + _max_size: int + _warn_size: int @classmethod def from_crawler(cls, crawler: "Crawler", *args: Any, **kwargs: Any) -> "Self": @@ -97,7 +99,7 @@ class SitemapSpider(Spider): body = gunzip(response.body, max_size=max_size) except _DecompressionMaxSizeExceeded: return None - if uncompressed_size < warn_size and len(body) >= warn_size: + if uncompressed_size < warn_size <= len(body): logger.warning( f"{response} body size after decompression ({len(body)} B) " f"is larger than the download warning size ({warn_size} B)." From 8e25f8c157e53c9f5df51e950fed18becfe7797d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 23 Nov 2023 14:12:59 +0100 Subject: [PATCH 12/20] Fix bad message --- scrapy/downloadermiddlewares/httpcompression.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 6c8b659bd..f03294d65 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -91,7 +91,7 @@ class HttpCompressionMiddleware: raise IgnoreRequest( f"Ignored response {response} because its body " f"({len(response.body)} B) exceeded DOWNLOAD_MAXSIZE " - f"({self._max_size} B) during decompression." + f"({max_size} B) during decompression." ) if len(response.body) < warn_size <= len(decoded_body): logger.warning( From 5f2827efe7b069e514a87bd0f7a9589f49f0a97a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 24 Nov 2023 10:02:56 +0100 Subject: [PATCH 13/20] Make HttpCompressionMiddleware changes backward-comaptible --- scrapy/downloadermiddlewares/httpcompression.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index f03294d65..1a3f6962a 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -37,8 +37,10 @@ class HttpCompressionMiddleware: """This middleware allows compressed (gzip, deflate) traffic to be sent/received from web sites""" - def __init__(self, *, crawler=None): + def __init__(self, stats=None, *, crawler=None): if not crawler: + if stats: + self.stats = stats return self.stats = crawler.stats self._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") From 6c278e1862c5453ec45a8f6a5c2472710895cad9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 24 Nov 2023 10:15:21 +0100 Subject: [PATCH 14/20] =?UTF-8?q?List[bytes]=20=E2=86=92=20BytesIO?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scrapy/utils/_compression.py | 22 ++++++++++++---------- scrapy/utils/gz.py | 12 ++++++------ 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py index a70f6c275..b17fd7881 100644 --- a/scrapy/utils/_compression.py +++ b/scrapy/utils/_compression.py @@ -1,6 +1,5 @@ import zlib from io import BytesIO -from typing import List try: import brotli @@ -21,7 +20,7 @@ def _inflate(data: bytes, *, max_size: int = 0) -> bytes: decompressor = zlib.decompressobj() raw_decompressor = zlib.decompressobj(wbits=-15) input_stream = BytesIO(data) - output_list: List[bytes] = [] + output_stream = BytesIO() output_chunk = b"." decompressed_size = 0 CHUNK_SIZE = 8196 @@ -47,14 +46,15 @@ def _inflate(data: bytes, *, max_size: int = 0) -> bytes: f"({decompressed_size} B) exceed the specified maximum " f"({max_size} B)." ) - output_list.append(output_chunk) - return b"".join(output_list) + output_stream.write(output_chunk) + output_stream.seek(0) + return output_stream.read() def _unbrotli(data: bytes, *, max_size: int = 0) -> bytes: decompressor = brotli.Decompressor() input_stream = BytesIO(data) - output_list: List[bytes] = [] + output_stream = BytesIO() output_chunk = b"." decompressed_size = 0 CHUNK_SIZE = 8196 @@ -68,14 +68,15 @@ def _unbrotli(data: bytes, *, max_size: int = 0) -> bytes: f"({decompressed_size} B) exceed the specified maximum " f"({max_size} B)." ) - output_list.append(output_chunk) - return b"".join(output_list) + output_stream.write(output_chunk) + output_stream.seek(0) + return output_stream.read() def _unzstd(data: bytes, *, max_size: int = 0) -> bytes: decompressor = zstandard.ZstdDecompressor() stream_reader = decompressor.stream_reader(BytesIO(data)) - output_list: List[bytes] = [] + output_stream = BytesIO() output_chunk = b"." decompressed_size = 0 CHUNK_SIZE = 8196 @@ -88,5 +89,6 @@ def _unzstd(data: bytes, *, max_size: int = 0) -> bytes: f"({decompressed_size} B) exceed the specified maximum " f"({max_size} B)." ) - output_list.append(output_chunk) - return b"".join(output_list) + output_stream.write(output_chunk) + output_stream.seek(0) + return output_stream.read() diff --git a/scrapy/utils/gz.py b/scrapy/utils/gz.py index e5cf68d62..5d23e8f05 100644 --- a/scrapy/utils/gz.py +++ b/scrapy/utils/gz.py @@ -1,7 +1,6 @@ import struct from gzip import GzipFile from io import BytesIO -from typing import List from scrapy.http import Response @@ -14,7 +13,7 @@ def gunzip(data: bytes, *, max_size: int = 0) -> bytes: This is resilient to CRC checksum errors. """ f = GzipFile(fileobj=BytesIO(data)) - output_list: List[bytes] = [] + output_stream = BytesIO() chunk = b"." decompressed_size = 0 while chunk: @@ -23,8 +22,8 @@ def gunzip(data: bytes, *, max_size: int = 0) -> bytes: except (OSError, EOFError, struct.error): # complete only if there is some data, otherwise re-raise # see issue 87 about catching struct.error - # some pages are quite small so output_list is empty - if output_list: + # some pages are quite small so output_stream is empty + if output_stream: break raise decompressed_size += len(chunk) @@ -34,8 +33,9 @@ def gunzip(data: bytes, *, max_size: int = 0) -> bytes: f"({decompressed_size} B) exceed the specified maximum " f"({max_size} B)." ) - output_list.append(chunk) - return b"".join(output_list) + output_stream.write(chunk) + output_stream.seek(0) + return output_stream.read() def gzip_magic_number(response: Response) -> bool: From 62398e424c0a3d66d0bdd3908e47284cbe797ef9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 24 Nov 2023 10:20:14 +0100 Subject: [PATCH 15/20] =?UTF-8?q?CHUNK=5FSIZE:=208=20KiB=20=E2=86=92=2032?= =?UTF-8?q?=20KiB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scrapy/utils/_compression.py | 12 ++++++------ scrapy/utils/gz.py | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py index b17fd7881..5610595d3 100644 --- a/scrapy/utils/_compression.py +++ b/scrapy/utils/_compression.py @@ -12,6 +12,9 @@ except ImportError: pass +_CHUNK_SIZE = 65536 # 64 KiB + + class _DecompressionMaxSizeExceeded(ValueError): pass @@ -23,9 +26,8 @@ def _inflate(data: bytes, *, max_size: int = 0) -> bytes: output_stream = BytesIO() output_chunk = b"." decompressed_size = 0 - CHUNK_SIZE = 8196 while output_chunk: - input_chunk = input_stream.read(CHUNK_SIZE) + input_chunk = input_stream.read(_CHUNK_SIZE) try: output_chunk = decompressor.decompress(input_chunk) except zlib.error: @@ -57,9 +59,8 @@ def _unbrotli(data: bytes, *, max_size: int = 0) -> bytes: output_stream = BytesIO() output_chunk = b"." decompressed_size = 0 - CHUNK_SIZE = 8196 while output_chunk: - input_chunk = input_stream.read(CHUNK_SIZE) + input_chunk = input_stream.read(_CHUNK_SIZE) output_chunk = decompressor.process(input_chunk) decompressed_size += len(output_chunk) if max_size and decompressed_size > max_size: @@ -79,9 +80,8 @@ def _unzstd(data: bytes, *, max_size: int = 0) -> bytes: output_stream = BytesIO() output_chunk = b"." decompressed_size = 0 - CHUNK_SIZE = 8196 while output_chunk: - output_chunk = stream_reader.read(CHUNK_SIZE) + output_chunk = stream_reader.read(_CHUNK_SIZE) decompressed_size += len(output_chunk) if max_size and decompressed_size > max_size: raise _DecompressionMaxSizeExceeded( diff --git a/scrapy/utils/gz.py b/scrapy/utils/gz.py index 5d23e8f05..cf7316e82 100644 --- a/scrapy/utils/gz.py +++ b/scrapy/utils/gz.py @@ -4,7 +4,7 @@ from io import BytesIO from scrapy.http import Response -from ._compression import _DecompressionMaxSizeExceeded +from ._compression import _CHUNK_SIZE, _DecompressionMaxSizeExceeded def gunzip(data: bytes, *, max_size: int = 0) -> bytes: @@ -18,7 +18,7 @@ def gunzip(data: bytes, *, max_size: int = 0) -> bytes: decompressed_size = 0 while chunk: try: - chunk = f.read1(8196) + chunk = f.read1(_CHUNK_SIZE) except (OSError, EOFError, struct.error): # complete only if there is some data, otherwise re-raise # see issue 87 about catching struct.error From 8a73c6c90c5984292d39a0a9d4be86e792d81cb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 24 Nov 2023 10:25:01 +0100 Subject: [PATCH 16/20] Fix HttpCompressionMiddleware backward compatibility --- scrapy/downloadermiddlewares/httpcompression.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 1a3f6962a..58ca1017f 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -39,8 +39,9 @@ class HttpCompressionMiddleware: def __init__(self, stats=None, *, crawler=None): if not crawler: - if stats: - self.stats = stats + self.stats = stats + self._max_size = 1073741824 + self._warn_size = 33554432 return self.stats = crawler.stats self._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") From a113208a0643263fdd7198238bf9213f9148bc1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 24 Nov 2023 11:35:15 +0100 Subject: [PATCH 17/20] Fix BytesIO non-emptiness check --- 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 cf7316e82..2e487d88b 100644 --- a/scrapy/utils/gz.py +++ b/scrapy/utils/gz.py @@ -23,7 +23,7 @@ def gunzip(data: bytes, *, max_size: int = 0) -> bytes: # complete only if there is some data, otherwise re-raise # see issue 87 about catching struct.error # some pages are quite small so output_stream is empty - if output_stream: + if output_stream.getbuffer().nbytes > 0: break raise decompressed_size += len(chunk) From bb74badd1bd66c59a63268c52342507c76d290b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 11 Dec 2023 17:39:55 +0100 Subject: [PATCH 18/20] =?UTF-8?q?spider=20=E2=86=92=20mw?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scrapy/downloadermiddlewares/httpcompression.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 58ca1017f..816be25a1 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -61,12 +61,12 @@ class HttpCompressionMiddleware: "reimplement their 'from_crawler' method.", ScrapyDeprecationWarning, ) - spider = cls() - spider.stats = crawler.stats - spider._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") - spider._warn_size = crawler.settings.getint("DOWNLOAD_WARNSIZE") - crawler.signals.connect(spider.open_spider, signals.spider_opened) - return spider + mw = cls() + mw.stats = crawler.stats + mw._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") + mw._warn_size = crawler.settings.getint("DOWNLOAD_WARNSIZE") + crawler.signals.connect(mw.open_spider, signals.spider_opened) + return mw def open_spider(self, spider): if hasattr(spider, "download_maxsize"): From b9c4ee26d7816b483b6b52ad1e74c3d8d9e19c36 Mon Sep 17 00:00:00 2001 From: Laerte Pereira Date: Tue, 17 Oct 2023 17:49:22 -0300 Subject: [PATCH 19/20] Remove deprecated scrapy.downloadermiddlewares.decompression --- scrapy/downloadermiddlewares/decompression.py | 94 ------------------- ...test_downloadermiddleware_decompression.py | 53 ----------- 2 files changed, 147 deletions(-) delete mode 100644 scrapy/downloadermiddlewares/decompression.py delete mode 100644 tests/test_downloadermiddleware_decompression.py diff --git a/scrapy/downloadermiddlewares/decompression.py b/scrapy/downloadermiddlewares/decompression.py deleted file mode 100644 index 3b8702419..000000000 --- a/scrapy/downloadermiddlewares/decompression.py +++ /dev/null @@ -1,94 +0,0 @@ -""" This module implements the DecompressionMiddleware which tries to recognise -and extract the potentially compressed responses that may arrive. -""" - -import bz2 -import gzip -import logging -import tarfile -import zipfile -from io import BytesIO -from tempfile import mktemp -from warnings import warn - -from scrapy.exceptions import ScrapyDeprecationWarning -from scrapy.responsetypes import responsetypes - -warn( - "scrapy.downloadermiddlewares.decompression is deprecated", - ScrapyDeprecationWarning, - stacklevel=2, -) - - -logger = logging.getLogger(__name__) - - -class DecompressionMiddleware: - """This middleware tries to recognise and extract the possibly compressed - responses that may arrive.""" - - def __init__(self): - self._formats = { - "tar": self._is_tar, - "zip": self._is_zip, - "gz": self._is_gzip, - "bz2": self._is_bzip2, - } - - def _is_tar(self, response): - archive = BytesIO(response.body) - try: - tar_file = tarfile.open(name=mktemp(), fileobj=archive) - except tarfile.ReadError: - return - - body = tar_file.extractfile(tar_file.members[0]).read() - respcls = responsetypes.from_args(filename=tar_file.members[0].name, body=body) - return response.replace(body=body, cls=respcls) - - def _is_zip(self, response): - archive = BytesIO(response.body) - try: - zip_file = zipfile.ZipFile(archive) - except zipfile.BadZipFile: - return - - namelist = zip_file.namelist() - body = zip_file.read(namelist[0]) - respcls = responsetypes.from_args(filename=namelist[0], body=body) - return response.replace(body=body, cls=respcls) - - def _is_gzip(self, response): - archive = BytesIO(response.body) - try: - body = gzip.GzipFile(fileobj=archive).read() - except OSError: - return - - respcls = responsetypes.from_args(body=body) - return response.replace(body=body, cls=respcls) - - def _is_bzip2(self, response): - try: - body = bz2.decompress(response.body) - except OSError: - return - - respcls = responsetypes.from_args(body=body) - return response.replace(body=body, cls=respcls) - - def process_response(self, request, response, spider): - if not response.body: - return response - - for fmt, func in self._formats.items(): - new_response = func(response) - if new_response: - logger.debug( - "Decompressed response with format: %(responsefmt)s", - {"responsefmt": fmt}, - extra={"spider": spider}, - ) - return new_response - return response diff --git a/tests/test_downloadermiddleware_decompression.py b/tests/test_downloadermiddleware_decompression.py deleted file mode 100644 index 95739414e..000000000 --- a/tests/test_downloadermiddleware_decompression.py +++ /dev/null @@ -1,53 +0,0 @@ -from unittest import TestCase, main - -from scrapy.downloadermiddlewares.decompression import DecompressionMiddleware -from scrapy.http import Response, XmlResponse -from scrapy.spiders import Spider -from scrapy.utils.test import assert_samelines -from tests import get_testdata - - -def _test_data(formats): - uncompressed_body = get_testdata("compressed", "feed-sample1.xml") - test_responses = {} - for format in formats: - body = get_testdata("compressed", "feed-sample1." + format) - test_responses[format] = Response("http://foo.com/bar", body=body) - return uncompressed_body, test_responses - - -class DecompressionMiddlewareTest(TestCase): - test_formats = ["tar", "xml.bz2", "xml.gz", "zip"] - uncompressed_body, test_responses = _test_data(test_formats) - - def setUp(self): - self.mw = DecompressionMiddleware() - self.spider = Spider("foo") - - def test_known_compression_formats(self): - for fmt in self.test_formats: - rsp = self.test_responses[fmt] - new = self.mw.process_response(None, rsp, self.spider) - error_msg = f"Failed {fmt}, response type {type(new).__name__}" - assert isinstance(new, XmlResponse), error_msg - assert_samelines(self, new.body, self.uncompressed_body, fmt) - - def test_plain_response(self): - rsp = Response(url="http://test.com", body=self.uncompressed_body) - new = self.mw.process_response(None, rsp, self.spider) - assert new is rsp - assert_samelines(self, new.body, rsp.body) - - def test_empty_response(self): - rsp = Response(url="http://test.com", body=b"") - new = self.mw.process_response(None, rsp, self.spider) - assert new is rsp - assert not rsp.body - assert not new.body - - def tearDown(self): - del self.mw - - -if __name__ == "__main__": - main() From 12b10a7a6427c43968cc18d98a3ed3c6366eeabd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 13 Dec 2023 13:35:05 +0100 Subject: [PATCH 20/20] Cover scrapy.downloadermiddlewares.decompression in the release notes --- docs/news.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/news.rst b/docs/news.rst index c4081b99b..a12bda53f 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -16,6 +16,10 @@ Scrapy 2.11.1 (unreleased) .. _7j7m-v7m3-jqm7 security advisory: https://github.com/scrapy/scrapy/security/advisories/GHSA-7j7m-v7m3-jqm7 +- Also in relation with the `7j7m-v7m3-jqm7 security advisory`_, the + deprecated ``scrapy.downloadermiddlewares.decompression`` module has been + removed. + .. _release-2.11.0: @@ -2896,6 +2900,10 @@ Scrapy 1.8.4 (unreleased) to the decompressed response body. Please, see the `7j7m-v7m3-jqm7 security advisory`_ for more information. +- Also in relation with the `7j7m-v7m3-jqm7 security advisory`_, use of the + ``scrapy.downloadermiddlewares.decompression`` module is discouraged and + will trigger a warning. + .. _release-1.8.3: