diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index 576feae7e..5e16510e5 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -445,6 +445,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 @@ -614,7 +637,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 9d67e98bf..ea38f0182 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -64,15 +64,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: @@ -81,12 +81,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( @@ -104,6 +117,8 @@ class FilesStoreProtocol(Protocol): class FSFilesStore: + checksum_algorithm: str = "md5" + def __init__(self, basedir: str | PathLike[str]): basedir = _to_string(basedir) if "://" in basedir: @@ -136,7 +151,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} @@ -155,6 +170,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 @@ -198,11 +218,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( @@ -292,6 +313,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" @@ -322,13 +347,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 @@ -371,6 +397,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 @@ -421,7 +449,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 @@ -465,6 +493,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], @@ -497,6 +527,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 @@ -581,6 +614,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, @@ -709,7 +751,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 7186fc8de..80de670f2 100644 --- a/scrapy/pipelines/images.py +++ b/scrapy/pipelines/images.py @@ -18,7 +18,13 @@ from itemadapter import ItemAdapter from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning from scrapy.http import Request, Response from scrapy.http.request import NO_CALLBACK -from scrapy.pipelines.files import FilesPipeline, GCSFilesStore, S3FilesStore, _md5sum +from scrapy.pipelines.files import ( + FilesPipeline, + GCSFilesStore, + S3FilesStore, + _checksum, + _checksum_algorithm, +) from scrapy.pipelines.media import FileException from scrapy.utils.defer import ensure_awaitable from scrapy.utils.python import to_bytes @@ -88,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"): @@ -154,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 928f4e2c2..dbe9f4504 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 mimetypes import random @@ -212,6 +213,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" @@ -386,6 +418,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 @@ -821,6 +875,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) == {} @@ -951,6 +1012,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: @@ -1095,6 +1181,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 dde555892..7d8d18c18 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.misc import build_from_crawler from scrapy.utils.test import get_crawler @@ -235,11 +236,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