diff --git a/CHANGELOG.md b/CHANGELOG.md index 357737d..b3a4663 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,12 @@ ### Fixed +- A large object (`format` `clob` or `blob`) whose body is JSON no longer raises `AttributeError: 'dict' object has + no attribute 'decode'`, or a `TypeError` for a blob: the API body handler parsed it as an envelope. A large object + is the scraped page itself, so only its compression is undone. Large objects are first resolved in 0.8.18; 0.8.17 + and earlier return the large-object URL string unresolved, with no warning. +- A large object the API answers with an error status raises its `ScrapflyError`, or an `HTTPError` when the answer + carries no API error, instead of being returned as the page. - Brotli webhooks decode with Google's `brotli`, and bodies past about 480 KB are no longer cut short. - Every webhook encoding is bounded per decoder call. A few KB of brotli or zstd could allocate gigabytes before the size limit applied, and crafted deflate or gzip bodies cost seconds of CPU, all before the diff --git a/scrapfly/api.py b/scrapfly/api.py index a10aa02..f767874 100644 --- a/scrapfly/api.py +++ b/scrapfly/api.py @@ -24,7 +24,7 @@ from .errors import * from .errors import ContentError from .reporter import Reporter -from .api_response import ResponseBodyHandler +from .api_response import ApiResponse, LargeObjectApiResponse, ResponseBodyHandler from .scrape_config import ScrapeConfig from .screenshot_config import ScreenshotConfig from .extraction_config import ExtractionConfig @@ -1292,15 +1292,10 @@ def _large_object_steps(self, callback_url:str, format: Literal['clob', 'blob']) params={'key': self.key} ) - if self.body_handler.support(headers=response.headers): - content = self.body_handler(content=response.content, content_type=response.headers['content-type']) - else: - content = response.content - - if format == 'clob': - return content.decode('utf-8'), 'text' + large_object = LargeObjectApiResponse(request=response.request, response=response, format=format, body_handler=self.body_handler) + large_object.raise_for_result() - return BytesIO(content), 'binary' + return large_object.content, large_object.content_format def _scrape_body(self, response: Response, scrape_config: ScrapeConfig): if scrape_config.method == 'HEAD': @@ -1310,26 +1305,7 @@ def _scrape_body(self, response: Response, scrape_config: ScrapeConfig): return self.body_handler(content=response.content, content_type=response.headers['content-type']) # body_handler rejected — content-type not in SUPPORTED_CONTENT_TYPES. - # Response may still be compressed (zstd/brotli) if requests did - # not transparently decompress. Probe content-encoding and try - # the handler's read() anyway before falling back to a tolerant - # utf-8 decode. - raw = response.content - content_encoding = response.headers.get('content-encoding', '').lower() - # urllib3 already inflated these: a second decode corrupts some bodies. - if content_encoding in ('gzip', 'gz', 'deflate', 'br', 'brotli', 'zstd') \ - and content_encoding not in urllib3.response.HTTPResponse.CONTENT_DECODERS: - try: - raw = self.body_handler.read( - content=raw, - content_encoding=content_encoding, - content_type=response.headers.get('content-type', ''), - signature=None, - ) - except Exception: - # Fall through to tolerant decode below; don't mask the - # real error with a decoder crash. - pass + raw = ApiResponse(request=response.request, response=response).decompress() if isinstance(raw, (bytes, bytearray)): return raw.decode('utf-8', errors='replace') diff --git a/scrapfly/api_response.py b/scrapfly/api_response.py index 9e85bbc..bd52743 100644 --- a/scrapfly/api_response.py +++ b/scrapfly/api_response.py @@ -21,8 +21,9 @@ from dateutil.parser import parse from requests import Request, Response, HTTPError -from typing import List, Dict, Optional, Iterable, Iterator, Union, TextIO, Tuple, Callable +from typing import List, Dict, Literal, Optional, Iterable, Iterator, Union, TextIO, Tuple, Callable +import urllib3 from requests.structures import CaseInsensitiveDict from .scrape_config import ScrapeConfig @@ -349,6 +350,23 @@ def __init__(self, request: Request, response: Response): self.request = request self.response = response + def decompress(self) -> bytes: + """The body, with a Content-Encoding the HTTP client left in place undone.""" + content = self.response.content + content_encoding = self.response.headers.get('content-encoding', '').lower() + + # urllib3 inflated these already: a second pass corrupts some bodies. + if content_encoding in urllib3.response.HTTPResponse.CONTENT_DECODERS: + return content + + try: + return decompress(content, content_encoding, MAX_DECOMPRESSED_SIZE) + except WebhookBodyTooLarge: + raise + except Exception: + # The client advertises only what it decodes: a body that does not decode was decoded already. + return content + @property def headers(self) -> CaseInsensitiveDict: return self.response.headers @@ -443,6 +461,36 @@ def raise_for_result( raise error_class(**args) from e +class LargeObjectApiResponse(ApiResponse): + """The clob or blob a scrape result points to: the scraped page itself, never an API envelope.""" + + def __init__(self, request: Request, response: Response, format: Literal['clob', 'blob'], body_handler: 'ResponseBodyHandler'): + super().__init__(request, response) + self.format = format + self.error = None + # Only an error answer is an API envelope. + self.result = {} + + if not response.ok and body_handler.support(headers=response.headers): + with suppress(EncoderError): + self.result = body_handler(content=response.content, content_type=response.headers['content-type']) + + def raise_for_result(self, raise_on_upstream_error: bool = True, error_class=ApiHttpClientError): + super().raise_for_result(raise_on_upstream_error=raise_on_upstream_error, error_class=error_class) + # An error answer without an envelope (a proxy's 502 page) is not the page either. + self.response.raise_for_status() + + @property + def content(self) -> Union[str, BytesIO]: + content = self.decompress() + + return content.decode('utf-8') if self.format == 'clob' else BytesIO(content) + + @property + def content_format(self) -> Literal['text', 'binary']: + return 'text' if self.format == 'clob' else 'binary' + + class ScrapeApiResponse(ApiResponse): scrape_config:ScrapeConfig large_object_handler:Callable diff --git a/tests/test_clients.py b/tests/test_clients.py index 24266b6..46609e6 100644 --- a/tests/test_clients.py +++ b/tests/test_clients.py @@ -1,7 +1,9 @@ """ScrapflyClient and AsyncScrapflyClient run the same operations: every scenario runs on both, against one fake API.""" import asyncio +import gzip import inspect import json +from io import BytesIO from dataclasses import dataclass from typing import Callable, Dict, List, Tuple from urllib.parse import parse_qs, urlsplit @@ -11,8 +13,9 @@ import requests from scrapfly import AsyncScrapflyClient, ExtractionConfig, ScrapeApiResponse, ScrapeConfig, ScrapflyClient -from scrapfly import async_client, client -from scrapfly.errors import CrawlerPromptError, ScrapflyError +from scrapfly import api_response, async_client, client +from scrapfly.api_response import ApiResponse +from scrapfly.errors import ApiHttpClientError, ApiHttpServerError, CrawlerPromptError, ScrapflyError pytestmark = pytest.mark.unit @@ -212,6 +215,128 @@ async def scenario(scrapfly): assert [s.path for s in seen] == ['/scrape', '/scrape/clob/01'] +@pytest.mark.parametrize('kind', CLIENTS) +@pytest.mark.parametrize('format,content_type,body,expected', [ + ('clob', 'application/json', b'{"products": [{"id": 1}]}', '{"products": [{"id": 1}]}'), + ('clob', 'text/html', b'

large

', '

large

'), + ('blob', 'application/pdf', b'%PDF-\x00\xff', b'%PDF-\x00\xff'), + ('blob', 'application/json', b'{"products": [{"id": 1}]}', b'{"products": [{"id": 1}]}'), +]) +def test_large_object_is_returned_as_is(kind, format, content_type, body, expected): + def server(seen: Seen): + if seen.path == '/scrape': + return json_reply(envelope(HOST + '/scrape/large/01', format=format)) + + return 200, {'content-type': content_type}, [body] + + async def scenario(scrapfly): + return await resolve(scrapfly.scrape(ScrapeConfig(url='https://web-scraping.dev/'))) + + response, _ = run(kind, server, scenario) + content = response.scrape_result['content'] + + # The envelope around the large object is still the API result. + assert response.config['url'] == 'https://web-scraping.dev/product/1' + assert response.scrape_result['success'] is True + + if format == 'clob': + assert isinstance(content, str) and content == expected + assert response.scrape_result['format'] == 'text' + else: + assert isinstance(content, BytesIO) and content.read() == expected + assert response.scrape_result['format'] == 'binary' + + +@pytest.mark.parametrize('kind', CLIENTS) +@pytest.mark.parametrize('status,headers,body,error', [ + (404, {'content-type': 'application/json'}, json.dumps({'reason': 'Not Found', 'message': 'Large Object 01 not found', 'error_id': '01', 'http_code': 404}).encode(), ApiHttpClientError), + (500, {'content-type': 'application/json'}, json.dumps({'reason': 'Internal Server Error', 'message': 'Internal Error', 'error_id': '01', 'http_code': 500}).encode(), ApiHttpServerError), + (502, {'content-type': 'text/html'}, b'

Bad Gateway

', requests.HTTPError), +]) +@pytest.mark.parametrize('format', ['clob', 'blob']) +def test_a_large_object_error_is_raised_not_returned_as_the_page(kind, status, headers, body, error, format): + def server(seen: Seen): + if seen.path == '/scrape': + return json_reply(envelope(HOST + '/scrape/large_object/01', format=format)) + + return status, headers, [body] + + async def scenario(scrapfly): + return await resolve(scrapfly.scrape(ScrapeConfig(url='https://web-scraping.dev/'))) + + with pytest.raises(error) as raised: + run(kind, server, scenario) + + assert KEY not in str(raised.value) + + +@pytest.mark.parametrize('content_encoding,body,expected', [ + ('gz', gzip.compress(b'large text'), b'large text'), + # The HTTP client inflated it and left the header: the body is returned as it is. + ('brotli', b'large text', b'large text'), + # A .gz file served gzip: urllib3 inflated one layer, the file is the content. + ('gzip', gzip.compress(b'large text'), gzip.compress(b'large text')), + ('', b'large text', b'large text'), +]) +def test_an_api_response_undoes_only_the_encoding_left_in_place(content_encoding, body, expected): + response = requests.Response() + response.status_code = 200 + response.headers['content-encoding'] = content_encoding + response._content = body + + assert ApiResponse(request=response.request, response=response).decompress() == expected + + +def test_an_api_response_past_the_decompression_limit_raises(monkeypatch): + monkeypatch.setattr(api_response, 'MAX_DECOMPRESSED_SIZE', 16) + response = requests.Response() + response.status_code = 200 + response.headers['content-encoding'] = 'gz' + response._content = gzip.compress(b'x' * 1024) + + with pytest.raises(api_response.WebhookBodyTooLarge): + ApiResponse(request=response.request, response=response).decompress() + + +@pytest.mark.parametrize('kind', CLIENTS) +def test_compressed_json_large_object_stays_text(kind): + # `gz` is gzip that urllib3 does not inflate. The SDK must, and must not parse the JSON. + page = b'{"products": [{"id": 1}]}' + + def server(seen: Seen): + if seen.path == '/scrape': + return json_reply(envelope(HOST + '/scrape/clob/02', format='clob')) + + return 200, {'content-type': 'application/json', 'content-encoding': 'gz'}, [gzip.compress(page)] + + async def scenario(scrapfly): + return await resolve(scrapfly.scrape(ScrapeConfig(url='https://web-scraping.dev/'))) + + response, _ = run(kind, server, scenario) + + assert response.scrape_result['content'] == page.decode() + assert response.scrape_result['format'] == 'text' + assert response.config['url'] == 'https://web-scraping.dev/product/1' + assert response.scrape_result['success'] is True + + +@pytest.mark.parametrize('kind', CLIENTS) +def test_a_json_scrape_envelope_is_still_parsed(kind): + """Sharing decompression with large objects must not skip parsing a real API envelope.""" + def server(seen: Seen): + return json_reply(envelope('{"products": [{"id": 1}]}')) + + async def scenario(scrapfly): + return await resolve(scrapfly.scrape(ScrapeConfig(url='https://web-scraping.dev/products.json'))) + + response, seen = run(kind, server, scenario) + + assert response.scrape_result['content'] == '{"products": [{"id": 1}]}' + assert response.scrape_result['format'] == 'text' + assert response.config['url'] == 'https://web-scraping.dev/product/1' + assert len(seen) == 1 + + @pytest.mark.parametrize('kind', CLIENTS) def test_a_dropped_connection_is_retried(kind, monkeypatch): drops = [DROP, DROP]