From ffcb43a63a481fc79b372ed0899452bcf3c90c24 Mon Sep 17 00:00:00 2001 From: Vishnu Kannaujia Date: Fri, 17 Jul 2026 17:40:04 -0700 Subject: [PATCH] Raise dedicated HashValidationError on download/extract hash mismatch Fixes #8832. Hash-validation failures in download_url and extractall previously raised a plain RuntimeError whose message contains "md5 check". The test helper skip_if_downloading_fails matched that substring and turned genuine hash mismatches (data corruption / content mismatch) into skipped tests, hiding real failures behind the same path as transient network errors. Introduce HashValidationError(RuntimeError) and raise it at all three hash checks (existing file, downloaded file, compressed file). Subclassing RuntimeError keeps existing `except RuntimeError` handlers working and the message text unchanged. skip_if_downloading_fails now re-raises HashValidationError instead of skipping, and the now-redundant "md5 check" entry is removed from DOWNLOAD_FAIL_MSGS. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Vishnu Kannaujia --- monai/apps/utils.py | 33 +++++++++++++++++++------ tests/apps/test_download_and_extract.py | 31 +++++++++++++++++++++++ tests/test_utils.py | 9 +++++-- 3 files changed, 64 insertions(+), 9 deletions(-) diff --git a/monai/apps/utils.py b/monai/apps/utils.py index 856bc64c9ea..5758c703fb5 100644 --- a/monai/apps/utils.py +++ b/monai/apps/utils.py @@ -42,12 +42,31 @@ else: tqdm, has_tqdm = optional_import("tqdm", "4.47.0", min_version, "tqdm") -__all__ = ["check_hash", "download_url", "extractall", "download_and_extract", "get_logger", "SUPPORTED_HASH_TYPES"] +__all__ = [ + "check_hash", + "download_url", + "extractall", + "download_and_extract", + "get_logger", + "SUPPORTED_HASH_TYPES", + "HashValidationError", +] DEFAULT_FMT = "%(asctime)s - %(levelname)s - %(message)s" SUPPORTED_HASH_TYPES = {"md5": hashlib.md5, "sha1": hashlib.sha1, "sha256": hashlib.sha256, "sha512": hashlib.sha512} +class HashValidationError(RuntimeError): + """ + Raised when a downloaded or existing file does not match its expected hash value. + + This is a dedicated subclass of :class:`RuntimeError` (so existing ``except RuntimeError`` + handlers keep working) that specifically signals data corruption or a content mismatch, + as opposed to a transient/incomplete download. It lets callers distinguish a genuine hash + failure from recoverable network errors and avoid silently skipping it. + """ + + def get_logger( module_name: str = "monai.apps", fmt: str = DEFAULT_FMT, @@ -213,14 +232,14 @@ def download_url( https://github.com/wkentaro/gdown/blob/main/gdown/download.py Raises: - RuntimeError: When the hash validation of the ``filepath`` existing file fails. + HashValidationError: When the hash validation of the ``filepath`` existing file fails. RuntimeError: When a network issue or denied permission prevents the file download from ``url`` to ``filepath``. URLError: See urllib.request.urlretrieve. HTTPError: See urllib.request.urlretrieve. ContentTooShortError: See urllib.request.urlretrieve. IOError: See urllib.request.urlretrieve. - RuntimeError: When the hash validation of the ``url`` downloaded file fails. + HashValidationError: When the hash validation of the ``url`` downloaded file fails. """ if not filepath: @@ -229,7 +248,7 @@ def download_url( filepath = Path(filepath) if filepath.exists(): if not check_hash(filepath, hash_val, hash_type): - raise RuntimeError( + raise HashValidationError( f"{hash_type} check of existing file failed: filepath={filepath}, expected {hash_type}={hash_val}." ) logger.info(f"File exists: {filepath}, skipped downloading.") @@ -268,7 +287,7 @@ def download_url( pass logger.info(f"Downloaded: {filepath}") if not check_hash(filepath, hash_val, hash_type): - raise RuntimeError( + raise HashValidationError( f"{hash_type} check of downloaded file failed: URL={url}, " f"filepath={filepath}, expected {hash_type}={hash_val}." ) @@ -325,7 +344,7 @@ def extractall( be False. Raises: - RuntimeError: When the hash validation of the ``filepath`` compressed file fails. + HashValidationError: When the hash validation of the ``filepath`` compressed file fails. NotImplementedError: When the ``filepath`` file extension is not one of [zip", "tar.gz", "tar"]. """ @@ -339,7 +358,7 @@ def extractall( return filepath = Path(filepath) if hash_val and not check_hash(filepath, hash_val, hash_type): - raise RuntimeError( + raise HashValidationError( f"{hash_type} check of compressed file failed: " f"filepath={filepath}, expected {hash_type}={hash_val}." ) logger.info(f"Writing into directory: {output_dir}.") diff --git a/tests/apps/test_download_and_extract.py b/tests/apps/test_download_and_extract.py index 6d16a727351..e39e8e166eb 100644 --- a/tests/apps/test_download_and_extract.py +++ b/tests/apps/test_download_and_extract.py @@ -21,6 +21,7 @@ from parameterized import parameterized from monai.apps import download_and_extract, download_url, extractall +from monai.apps.utils import HashValidationError from tests.test_utils import SkipIfNoModule, skip_if_downloading_fails, skip_if_quick, testing_data_config @@ -68,6 +69,36 @@ def test_default(self, key, file_type): ) +class TestHashValidationError(unittest.TestCase): + """A hash mismatch must raise the dedicated HashValidationError (offline, no network).""" + + def test_existing_file_wrong_hash_raises(self): + with tempfile.TemporaryDirectory() as tmp_dir: + filepath = Path(tmp_dir) / "data.bin" + filepath.write_bytes(b"monai") + # existing-file branch of download_url validates the on-disk file against hash_val. + with self.assertRaises(HashValidationError): + download_url("https://example.com/data.bin", filepath, hash_val="0" * 32, hash_type="md5") + + def test_extractall_wrong_hash_raises(self): + with tempfile.TemporaryDirectory() as tmp_dir: + zip_path = Path(tmp_dir) / "archive.zip" + with zipfile.ZipFile(zip_path, "w") as zf: + zf.writestr("a.txt", "content") + with self.assertRaises(HashValidationError): + extractall(str(zip_path), str(Path(tmp_dir) / "out"), hash_val="0" * 32, hash_type="md5") + + def test_hash_error_is_runtime_error_subclass(self): + # backward compatibility: existing ``except RuntimeError`` handlers keep catching it. + self.assertTrue(issubclass(HashValidationError, RuntimeError)) + + def test_skip_if_downloading_fails_does_not_suppress(self): + # a genuine hash mismatch must propagate, not be turned into a skipped test. + with self.assertRaises(HashValidationError): + with skip_if_downloading_fails(): + raise HashValidationError("md5 check of downloaded file failed: ...") + + class TestPathTraversalProtection(unittest.TestCase): """Test cases for path traversal attack protection in extractall function.""" diff --git a/tests/test_utils.py b/tests/test_utils.py index 5e21e48068b..0e50f711572 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -41,7 +41,7 @@ import torch import torch.distributed as dist -from monai.apps.utils import download_url +from monai.apps.utils import HashValidationError, download_url from monai.config import NdarrayTensor from monai.config.deviceconfig import USE_COMPILED from monai.config.type_definitions import NdarrayOrTensor @@ -81,7 +81,6 @@ "unexpected EOF", # incomplete download "network issue", "gdown dependency", # gdown not installed - "md5 check", "limit", # HTTP Error 503: Egress is over the account limit "authenticate", "timed out", # urlopen error [Errno 110] Connection timed out @@ -169,10 +168,16 @@ def assert_allclose( def skip_if_downloading_fails(): """ Skips a test if downloading something raises an exception recognised to indicate a download has failed. + + A :class:`~monai.apps.utils.HashValidationError` is deliberately *not* treated as a recoverable + download failure: it indicates the downloaded content does not match its expected hash (data + corruption or a content mismatch), so it is re-raised instead of skipping the test. """ try: yield + except HashValidationError: + raise # genuine hash mismatch, surface it rather than silently skipping except DOWNLOAD_EXCEPTS as e: raise unittest.SkipTest(f"Error while downloading: {e}") from e except ssl.SSLError as ssl_e: