Don't cache HTTP error responses in the remote file cache

jeandet · claude · jeandet · commit 0bbe2a5c6438 · 2026-06-12T16:50:17.000+02:00
_cached_get_remote_file stored resp.bytes without checking the HTTP
status (unlike its non-cached sibling _remote_open): a transient
502 Bad Gateway page got cached as the file content, permanently
poisoning the cache for that URL — every later read handed the HTML
error page to the consumer (e.g. netCDF4 failing with
"Unknown file format" long after the server recovered).

Raise IOError on non-200 responses before touching the cache, exactly
like _remote_open does.

Co-Authored-By: Claude Fable 5 &lt;noreply@anthropic.com&gt;
diff --git a/speasy/core/any_files.py b/speasy/core/any_files.py
@@ -83,6 +83,10 @@ def _cached_get_remote_file(url, timeout: int = http.DEFAULT_TIMEOUT, headers: d
         entry = get_item(url)
         if not isinstance(entry, CacheItem) or (not prefer_cache and _is_outdated(entry, url)):
             resp = http.urlopen(url=url, headers=headers, timeout=timeout)
+            if resp.status != 200:
+                # Never cache error responses: a transient 502 page stored as
+                # the file content poisons the cache until manually purged.
+                raise IOError(f"Could not open remote file {url}: HTTP {resp.status}")
             last_modified = resp.headers.get('last-modified', str(datetime.now()))
             if 'b' in mode:
                 entry = CacheItem(data=resp.bytes, version=last_modified)
diff --git a/tests/test_file_access.py b/tests/test_file_access.py
@@ -11,8 +11,9 @@
 from ddt import ddt, data, unpack
 
 from speasy.core.any_files import any_loc_open, list_files
-from speasy.core.cache import drop_item
+from speasy.core.cache import drop_item, get_item
 from multiprocessing import Value, Process
+from unittest.mock import patch, MagicMock
 
 _HERE_ = os.path.dirname(os.path.abspath(__file__))
 
@@ -65,6 +66,22 @@ def test_simple_remote_bin_file_with_rewrite_rules(self):
         self.assertIsNotNone(f)
         self.assertIn(b'NSSDC Common Data Format', f.read(100))
 
+    def test_http_error_responses_raise_and_are_not_cached(self):
+        # A transient 502 used to be stored in the cache as the file content,
+        # permanently poisoning it: every later read handed the HTML error
+        # page to the consumer (e.g. netCDF4 -> "Unknown file format").
+        url = "https://test.invalid/some_file_behind_flaky_server.nc"
+        drop_item(url)
+        error_resp = MagicMock()
+        error_resp.status = 502
+        error_resp.bytes = b"<html>502 Bad Gateway</html>"
+        error_resp.text = "<html>502 Bad Gateway</html>"
+        error_resp.headers = {}
+        with patch("speasy.core.any_files.http.urlopen", return_value=error_resp):
+            with self.assertRaises(IOError):
+                any_loc_open(url, mode='rb', cache_remote_files=True)
+        self.assertIsNone(get_item(url), "HTTP error body must never be cached")
+
     def test_cached_remote_bin_file(self):
         drop_item("https://hephaistos.lpp.polytechnique.fr/data/LFR/SW/LFR-FSW/3.0.0.0/fsw")
         start = datetime.now()
