diff --git a/monai/apps/utils.py b/monai/apps/utils.py index 856bc64c9e..5758c703fb 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 6d16a72735..e39e8e166e 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 5e21e48068..0e50f71157 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: