From 6f79d304ad49a106026db8431dc5fdab54843d73 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Sat, 8 Aug 2026 22:48:42 +0200 Subject: [PATCH] Make the media pipeline checksum algorithm configurable --- docs/topics/media-pipeline.rst | 26 +++++++- scrapy/pipelines/files.py | 76 +++++++++++++++++----- scrapy/pipelines/images.py | 8 ++- tests/test_pipeline_files.py | 111 ++++++++++++++++++++++++++++++++- tests/test_pipeline_images.py | 19 +++++- 5 files changed, 216 insertions(+), 24 deletions(-) diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index b16066d0c..b11b53b5e 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -414,6 +414,29 @@ and pipeline class MyPipeline will have expiration time set to 180. The last modified time from the file is used to determine the age of the file in days, which is then compared to the set expiration time to determine if the file is expired. +.. _checksum-algorithm: + +Checksum algorithm +------------------ + +.. setting:: FILES_CHECKSUM_ALGORITHM +.. setting:: IMAGES_CHECKSUM_ALGORITHM + +.. versionadded:: VERSION + +Checksums are `MD5 hashes `_ by default. To use a different +algorithm, set :setting:`FILES_CHECKSUM_ALGORITHM` (or +:setting:`IMAGES_CHECKSUM_ALGORITHM`, in case of the Images Pipeline) to the +name of any algorithm supported by :func:`hashlib.new`: + +.. code-block:: python + + FILES_CHECKSUM_ALGORITHM = "sha256" + +Amazon S3 and Google Cloud Storage cannot calculate checksums of stored files +on demand, and only report MD5 hashes, so with a different algorithm files that +are not downloaded again, because they have not expired yet, get no checksum. + .. _topics-images-thumbnails: Thumbnail generation for images @@ -583,7 +606,8 @@ See here the methods that you can override in your custom Files Pipeline: * ``path`` - the path (relative to :setting:`FILES_STORE`) where the file was stored - * ``checksum`` - a `MD5 hash`_ of the image contents + * ``checksum`` - a `MD5 hash`_ of the file contents, unless a different + :ref:`checksum algorithm ` is configured * ``status`` - the file status indication. diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 55a3676e5..9b6c6057e 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -66,15 +66,15 @@ def _to_string(path: str | PathLike[str]) -> str: return str(path) # convert a Path object to string -def _md5sum(file: IO[bytes]) -> str: - """Calculate the md5 checksum of a file-like object without reading its +def _checksum(file: IO[bytes], algorithm: str = "md5") -> str: + """Calculate the checksum of a file-like object without reading its whole content in memory. >>> from io import BytesIO - >>> _md5sum(BytesIO(b'file content to hash')) + >>> _checksum(BytesIO(b'file content to hash')) '784406af91dd5a54fbb9c84c2236595a' """ - m = hashlib.md5() # noqa: S324 + m = hashlib.new(algorithm) while True: d = file.read(8096) if not d: @@ -83,12 +83,25 @@ def _md5sum(file: IO[bytes]) -> str: return m.hexdigest() +def _checksum_algorithm(settings: BaseSettings, key: str) -> str: + algorithm: str = settings.get(key, "md5") + hashlib.new(algorithm) # fail early on an unsupported algorithm + return algorithm + + class StatInfo(TypedDict, total=False): checksum: str last_modified: float class FilesStoreProtocol(Protocol): + checksum_algorithm: str + """Name of the :mod:`hashlib` algorithm that :meth:`stat_file` must use for + the checksums that it calculates itself. + + It is assigned by the pipeline. + """ + def __init__(self, basedir: str): ... def persist_file( @@ -106,6 +119,8 @@ class FilesStoreProtocol(Protocol): class FSFilesStore: + checksum_algorithm: str = "md5" + def __init__(self, basedir: str | PathLike[str]): basedir = _to_string(basedir) if "://" in basedir: @@ -138,7 +153,7 @@ class FSFilesStore: return {} with absolute_path.open("rb") as f: - checksum = _md5sum(f) + checksum = _checksum(f, self.checksum_algorithm) return {"last_modified": last_modified, "checksum": checksum} @@ -157,6 +172,11 @@ class FSFilesStore: class S3FilesStore: + # Amazon S3 cannot hash a stored object on demand. It reports the ETag, + # which is an MD5 hash for the objects that persist_file() uploads, and the + # checksums that were requested when the object was uploaded. + checksum_algorithm: str = "md5" + AWS_ACCESS_KEY_ID = None AWS_SECRET_ACCESS_KEY = None AWS_SESSION_TOKEN = None @@ -200,11 +220,12 @@ class S3FilesStore: raise ValueError(f"Incorrect URI scheme in {uri}, expected 's3'") self.bucket, self.prefix = uri[5:].split("/", 1) - @staticmethod - def _onsuccess(boto_key: dict[str, Any]) -> StatInfo: - checksum = boto_key["ETag"].strip('"') + def _onsuccess(self, boto_key: dict[str, Any]) -> StatInfo: last_modified = boto_key["LastModified"] modified_stamp = time.mktime(last_modified.timetuple()) + if self.checksum_algorithm != "md5": + return {"last_modified": modified_stamp} + checksum = boto_key["ETag"].strip('"') return {"checksum": checksum, "last_modified": modified_stamp} def stat_file( @@ -294,6 +315,10 @@ class S3FilesStore: class GCSFilesStore: + # Google Cloud Storage cannot hash a stored object on demand. It reports an + # MD5 hash and a CRC32C checksum. + checksum_algorithm: str = "md5" + GCS_PROJECT_ID = None CACHE_CONTROL = "max-age=172800" @@ -324,13 +349,14 @@ class GCSFilesStore: {"bucket": bucket}, ) - @staticmethod - def _onsuccess(blob: Any) -> StatInfo: - if blob: - checksum = base64.b64decode(blob.md5_hash).hex() - last_modified = time.mktime(blob.updated.timetuple()) - return {"checksum": checksum, "last_modified": last_modified} - return {} + def _onsuccess(self, blob: Any) -> StatInfo: + if not blob: + return {} + last_modified = time.mktime(blob.updated.timetuple()) + if self.checksum_algorithm != "md5": + return {"last_modified": last_modified} + checksum = base64.b64decode(blob.md5_hash).hex() + return {"checksum": checksum, "last_modified": last_modified} def stat_file( self, path: str, info: MediaPipeline.SpiderInfo @@ -373,6 +399,8 @@ class GCSFilesStore: class FTPFilesStore: + checksum_algorithm: str = "md5" + FTP_USERNAME: str | None = None FTP_PASSWORD: str | None = None USE_ACTIVE_MODE: bool | None = None @@ -423,7 +451,7 @@ class FTPFilesStore: ftp.set_pasv(False) file_path = f"{self.basedir}/{path}" last_modified = float(ftp.voidcmd(f"MDTM {file_path}")[4:].strip()) - m = hashlib.md5() # noqa: S324 + m = hashlib.new(self.checksum_algorithm) ftp.retrbinary(f"RETR {file_path}", m.update) return {"last_modified": last_modified, "checksum": m.hexdigest()} # The file doesn't exist @@ -467,6 +495,8 @@ class FilesPipeline(MediaPipeline): DEFAULT_FILES_URLS_FIELD: str = "file_urls" DEFAULT_FILES_RESULT_FIELD: str = "files" + _checksum_warned: bool = False + def __init__( self, store_uri: str | PathLike[str], @@ -499,6 +529,9 @@ class FilesPipeline(MediaPipeline): resolve = functools.partial( self._key_for_pipe, base_class_name=cls_name, settings=settings ) + self.store.checksum_algorithm = _checksum_algorithm( + settings, resolve("FILES_CHECKSUM_ALGORITHM") + ) self.expires: int = settings.getint(resolve("FILES_EXPIRES"), self.EXPIRES) if not hasattr(self, "FILES_URLS_FIELD"): self.FILES_URLS_FIELD = self.DEFAULT_FILES_URLS_FIELD @@ -583,6 +616,15 @@ class FilesPipeline(MediaPipeline): self.inc_stats("uptodate") checksum = result.get("checksum", None) + if checksum is None and not self._checksum_warned: + self._checksum_warned = True + logger.warning( + f"{self.store.__class__.__name__} does not report " + f"{self.store.checksum_algorithm} checksums of stored files, so " + f"files that are not downloaded again, because they have not " + f"expired yet, get no checksum.", + extra={"spider": info.spider}, + ) return { "url": request.url, "path": path, @@ -711,7 +753,7 @@ class FilesPipeline(MediaPipeline): ) -> str: path = self.file_path(request, response=response, info=info, item=item) buf = BytesIO(response.body) - checksum = _md5sum(buf) + checksum = _checksum(buf, self.store.checksum_algorithm) buf.seek(0) await ensure_awaitable(self.store.persist_file(path, buf, info)) return checksum diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py index 79b6c4f27..91246d856 100644 --- a/scrapy/pipelines/images.py +++ b/scrapy/pipelines/images.py @@ -23,7 +23,8 @@ from scrapy.pipelines.files import ( FilesPipeline, GCSFilesStore, S3FilesStore, - _md5sum, + _checksum, + _checksum_algorithm, ) from scrapy.utils.defer import ensure_awaitable from scrapy.utils.python import to_bytes @@ -93,6 +94,9 @@ class ImagesPipeline(FilesPipeline): base_class_name="ImagesPipeline", settings=settings, ) + self.store.checksum_algorithm = _checksum_algorithm( + settings, resolve("IMAGES_CHECKSUM_ALGORITHM") + ) self.expires: int = settings.getint(resolve("IMAGES_EXPIRES"), self.EXPIRES) if not hasattr(self, "IMAGES_RESULT_FIELD"): @@ -159,7 +163,7 @@ class ImagesPipeline(FilesPipeline): for path, image, buf in self.get_images(response, request, info, item=item): if checksum is None: buf.seek(0) - checksum = _md5sum(buf) + checksum = _checksum(buf, self.store.checksum_algorithm) width, height = image.size await ensure_awaitable( self.store.persist_file( diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 4e7fb118b..c7f045308 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -1,5 +1,6 @@ import base64 import dataclasses +import hashlib import logging import random import re @@ -95,8 +96,12 @@ class TestFilesPipeline: def teardown_method(self): rmtree(self.tempdir) - def _create_pipeline(self, pipeline_cls: type[FilesPipeline]) -> FilesPipeline: - crawler = get_crawler(DefaultSpider, {"FILES_STORE": self.tempdir}) + def _create_pipeline( + self, pipeline_cls: type[FilesPipeline], settings: dict[str, Any] | None = None + ) -> FilesPipeline: + crawler = get_crawler( + DefaultSpider, {"FILES_STORE": self.tempdir, **(settings or {})} + ) crawler.spider = crawler._create_spider() crawler.engine = MagicMock(download_async=mocked_download_func) pipeline = pipeline_cls.from_crawler(crawler) @@ -203,6 +208,37 @@ class TestFilesPipeline: assert result["files"][0]["checksum"] == "abc" assert result["files"][0]["status"] == "uptodate" + @coroutine_test + async def test_file_not_expired_without_checksum( + self, caplog: pytest.LogCaptureFixture + ) -> None: + """Stores that do not report checksums, e.g. those that cannot report + them for the configured algorithm, trigger a single warning.""" + with ( + mock.patch.object(FilesPipeline, "inc_stats", return_value=True), + mock.patch.object( + FSFilesStore, "stat_file", return_value={"last_modified": time.time()} + ), + caplog.at_level(logging.WARNING), + ): + for item_url in ("http://example.com/1.pdf", "http://example.com/2.pdf"): + with mock.patch.object( + FilesPipeline, + "get_media_requests", + return_value=[_prepare_request_object(item_url)], + ): + result = await self.pipeline.process_item( + _create_item_with_files(item_url) + ) + assert result["files"][0]["checksum"] is None + records = [ + r + for r in caplog.records + if "does not report md5 checksums" in r.getMessage() + ] + assert len(records) == 1 + assert "FSFilesStore" in records[0].getMessage() + @coroutine_test async def test_file_expired(self): item_url = "http://example.com/file2.pdf" @@ -377,6 +413,28 @@ class TestFilesPipeline: assert path.exists() assert path.read_bytes() == b"data" + @coroutine_test + async def test_checksum_algorithm(self) -> None: + pipeline = self._create_pipeline( + FilesPipeline, {"FILES_CHECKSUM_ALGORITHM": "sha256"} + ) + item_url = "http://example.com/file.pdf" + item = _create_item_with_files(item_url) + with ( + mock.patch.object(FilesPipeline, "inc_stats", return_value=True), + mock.patch.object( + FilesPipeline, + "get_media_requests", + return_value=[_prepare_request_object(item_url)], + ), + ): + result = await pipeline.process_item(item) + assert result["files"][0]["checksum"] == hashlib.sha256(b"data").hexdigest() + + def test_checksum_algorithm_unsupported(self) -> None: + with pytest.raises(ValueError, match="unsupported hash type"): + self._create_pipeline(FilesPipeline, {"FILES_CHECKSUM_ALGORITHM": "sha257"}) + def test_file_path_from_item(self): """ Custom file path based on item data, overriding default implementation @@ -763,6 +821,13 @@ class TestFSFilesStore: assert stat["checksum"] == "8d777f385d3dfec8815d20f7496026dc" assert stat["last_modified"] == pytest.approx(time.time(), abs=60) + def test_stat_file_checksum_algorithm(self, tmp_path: Path) -> None: + store = FSFilesStore(tmp_path) + store.checksum_algorithm = "sha256" + store.persist_file("full/filename", BytesIO(b"data"), DUMMY_SPIDER_INFO) + stat = store.stat_file("full/filename", DUMMY_SPIDER_INFO) + assert stat["checksum"] == hashlib.sha256(b"data").hexdigest() + def test_stat_missing_file(self, tmp_path: Path) -> None: store = FSFilesStore(tmp_path) assert store.stat_file("full/filename", DUMMY_SPIDER_INFO) == {} @@ -893,6 +958,31 @@ class TestS3FilesStore: "last_modified": last_modified.timestamp(), } + @inline_callbacks_test + def test_stat_checksum_algorithm(self): + """Amazon S3 only reports MD5 checksums, so no checksum is reported + when a different algorithm is configured.""" + bucket = "mybucket" + key = "export.csv" + last_modified = datetime(2019, 12, 1) + + store = S3FilesStore(f"s3://{bucket}/{key}") + store.checksum_algorithm = "sha256" + from botocore.stub import Stubber # noqa: PLC0415 + + with Stubber(store.s3_client) as stub: + stub.add_response( + "head_object", + expected_params={"Bucket": bucket, "Key": key}, + service_response={ + "ETag": '"3187896a9657a28163abb31667df64c8"', + "LastModified": last_modified, + }, + ) + + file_stats = yield store.stat_file("", info=DUMMY_SPIDER_INFO) + assert file_stats == {"last_modified": last_modified.timestamp()} + stub.assert_no_pending_responses() def test_default_max_pool_connections(self) -> None: @@ -1037,6 +1127,23 @@ class TestGCSFilesStore: "last_modified": time.mktime(updated.timetuple()), } + @coroutine_test + async def test_stat_checksum_algorithm(self) -> None: + """Google Cloud Storage only reports MD5 checksums, so no checksum is + reported when a different algorithm is configured.""" + store, bucket, blob = self.build_gcs_files_store() + store.checksum_algorithm = "sha256" + blob.md5_hash = base64.b64encode( + bytes.fromhex("cdcda85605e46d0af6110752770dce3c") + ).decode() + updated = datetime(2019, 12, 1) + blob.updated = updated + bucket.get_blob.return_value = blob + stat = await maybe_deferred_to_future( + store.stat_file("full/filename", info=DUMMY_SPIDER_INFO) + ) + assert stat == {"last_modified": time.mktime(updated.timetuple())} + @coroutine_test async def test_stat_missing_blob(self) -> None: store, bucket, _ = self.build_gcs_files_store() diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index 19e61579f..2aa42350d 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -1,6 +1,7 @@ from __future__ import annotations import dataclasses +import hashlib import io import random import sys @@ -18,7 +19,7 @@ from itemadapter import ItemAdapter from scrapy.exceptions import NotConfigured from scrapy.http import Request, Response from scrapy.item import Field, Item -from scrapy.pipelines.files import GCSFilesStore, S3FilesStore, _md5sum +from scrapy.pipelines.files import GCSFilesStore, S3FilesStore, _checksum from scrapy.pipelines.images import ImageException, ImagesPipeline from scrapy.utils.test import get_crawler from tests.utils.decorators import coroutine_test @@ -222,11 +223,25 @@ class TestImagesPipeline: ) buf.seek(0) - assert checksum == _md5sum(buf) + assert checksum == _checksum(buf) name = "3fd165099d8e71b8a48b2683946e64dbfad8b52d.jpg" assert Path(self.tempdir, "full", name).read_bytes() == buf.getvalue() assert Path(self.tempdir, "thumbs", "small", name).exists() + @coroutine_test + async def test_image_downloaded_checksum_algorithm(self) -> None: + crawler = get_crawler(settings_dict={"IMAGES_CHECKSUM_ALGORITHM": "sha256"}) + pipeline = ImagesPipeline(self.tempdir, crawler=crawler) + _, buf = _create_image("JPEG", "RGB", (50, 50), (0, 0, 0)) + url = "https://dev.mydeco.com/mydeco.gif" + response = Response(url=url, body=buf.getvalue()) + + checksum = await pipeline.image_downloaded( + response, Request(url=url), DUMMY_SPIDER_INFO + ) + + assert checksum == hashlib.sha256(buf.getvalue()).hexdigest() + def test_convert_image(self): SIZE = (100, 100) # straight forward case: RGB and JPEG