DM-56138: Fix detection of byte range support for http

doc/changes/DM-56138.bugfix.rst

@@ -0,0 +1 @@
+Opening a presigned S3 URL over HTTP no longer downloads the entire object into memory.

doc/changes/DM-56138.perf.rst

@@ -0,0 +1,2 @@
+``ResourcePath.open()`` on a plain HTTP URL now passes the resource size already reported by the range-support probe to the returned handle.
+Seeking relative to the end of the resource no longer needs an extra request to ask the server for its size.

python/lsst/resources/_resourceHandles/_httpResourceHandle.py

@@ -57,6 +57,10 @@ class HttpReadResourceHandle(BaseResourceHandle[bytes]):
         Defaults to newline. If a file is opened in binary mode, this argument
         is not used, as binary files will only split lines on the binary
         newline representation.
+    size : `int` or `None`, optional
+        Total size of the remote resource in bytes, if it is already known.
+        Saves a request to the server the first time the size is needed, for
+        example when seeking relative to the end of the resource.
     """
 
     def __init__(
@@ -67,6 +71,7 @@ def __init__(
         *,
         timeout: tuple[float, float] | None = None,
         newline: AnyStr | None = None,
+        size: int | None = None,
     ) -> None:
         super().__init__(mode, log, uri, newline=newline)
         self._url = uri.geturl()
@@ -81,7 +86,7 @@ def __init__(
         self._closed = CloseStatus.OPEN
         self._current_position = 0
         self._eof = False
-        self._total_size = -1  # Unknown
+        self._total_size = -1 if size is None else size  # -1 means unknown
 
     def close(self) -> None:
         self._closed = CloseStatus.CLOSED

python/lsst/resources/gs.py

@@ -108,7 +108,7 @@ def is_retryable(exc: Exception) -> bool:
     return isinstance(exc, _RETRIEVABLE_TYPES)
 
 
-_RETRY_POLICY = retry.Retry(predicate=is_retryable) if retry else None
+_RETRY_POLICY = retry.Retry(predicate=is_retryable) if retry is not None else None
 
 
 _client = None

python/lsst/resources/http.py

@@ -2052,10 +2052,16 @@ def _openImpl(
         encoding: str | None = None,
     ) -> Generator[ResourceHandleProtocol]:
         resp = self._head()
-        accepts_range = resp.status_code == requests.codes.ok and resp.headers.get("Accept-Ranges") == "bytes"
+        # A presigned S3 URL is signed for a single method, so _head() emulates
+        # HEAD with a one-byte ranged GET, which is answered with 206 rather
+        # than 200.
+        range_capable = (requests.codes.ok, requests.codes.partial_content)
+        accepts_range = resp.status_code in range_capable and resp.headers.get("Accept-Ranges") == "bytes"
         handle: ResourceHandleProtocol
         if mode in ("rb", "r") and accepts_range:
-            handle = HttpReadResourceHandle(mode, log, self, timeout=self._config.timeout)
+            handle = HttpReadResourceHandle(
+                mode, log, self, timeout=self._config.timeout, size=_total_size_from_partial_content(resp)
+            )
             if mode == "r":
                 # cast because the protocol is compatible, but does not have
                 # BytesIO in the inheritance tree
@@ -2071,6 +2077,29 @@ def _copy_extra_attributes(self, original_uri: ResourcePath) -> None:
         self._extra_headers = original_uri._extra_headers
 
 
+def _total_size_from_partial_content(resp: requests.Response) -> int | None:
+    """Return the total size of the resource reported by a 206 response.
+
+    Parameters
+    ----------
+    resp : `requests.Response`
+        Response to inspect.
+
+    Returns
+    -------
+    size : `int` or `None`
+        Total size of the resource in bytes, or `None` if the response does
+        not report one. A 200 response is deliberately ignored because its
+        'Content-Length' describes the transferred body, which may be
+        content-encoded, rather than the resource itself.
+    """
+    if resp.status_code != requests.codes.partial_content:
+        return None
+    if (content_range_header := resp.headers.get("Content-Range")) is None:
+        return None
+    return parse_content_range_header(content_range_header).total
+
+
 def _dump_response(resp: requests.Response) -> None:
     """Log the contents of a HTTP or webDAV request and its response.
 

tests/test_http.py

@@ -14,6 +14,7 @@
 import os.path
 import pickle
 import random
+import re
 import shutil
 import socket
 import stat
@@ -155,6 +156,93 @@ def test_get_info(self):
         self.assertEqual(info.checksums, {"md5": "rL0Y20zC+Fzt72VPzMSk2A==", "sha-256": "def456"})
         self.assertEqual(len(responses.calls), 2)
 
+    @responses.activate
+    def test_open_presigned_s3_url_uses_range_requests(self):
+        """Opening a presigned S3 URL must read byte ranges on demand rather
+        than downloading the whole object.
+        """
+        _get_dav_and_server_headers.cache_clear()
+        responses.add(responses.OPTIONS, "http://s3.test/", status=200)
+
+        body = b"0123456789abcdef"
+        url = "http://s3.test/big.dat?AWSAccessKeyId=key&Signature=sig&Expires=1000"
+
+        def serve_range(request):
+            # A presigned URL is signed for GET only, so the HEAD used to probe
+            # for range support is emulated with a one-byte ranged GET. Such a
+            # request is answered with 206, never 200.
+            byte_range = request.headers.get("Range")
+            if byte_range is None:
+                return (200, {"Accept-Ranges": "bytes"}, body)
+            # An open-ended range such as "bytes=4-" runs to the end of body.
+            start, end = re.fullmatch(r"bytes=(\d+)-(\d*)", byte_range).groups()
+            first = int(start)
+            if first >= len(body):
+                return (416, {"Accept-Ranges": "bytes"}, b"")
+            last = int(end) if end else len(body) - 1
+            chunk = body[first : last + 1]
+            return (
+                206,
+                {
+                    "Accept-Ranges": "bytes",
+                    "Content-Range": f"bytes {first}-{first + len(chunk) - 1}/{len(body)}",
+                },
+                chunk,
+            )
+
+        responses.add_callback(responses.GET, url, callback=serve_range)
+
+        with ResourcePath(url).open("rb") as handle:
+            self.assertIsInstance(handle, HttpReadResourceHandle)
+            handle.seek(-4, io.SEEK_END)
+            self.assertEqual(handle.read(), b"cdef")
+
+        gets = [call.request for call in responses.calls if call.request.method == "GET"]
+        self.assertTrue(gets)
+        for request in gets:
+            self.assertIn("Range", request.headers)
+
+    @responses.activate
+    def test_open_presigned_s3_url_reuses_probed_size(self):
+        """The one-byte GET used to probe for range support already reports the
+        total size, so the handle must not ask the server for it again.
+        """
+        _get_dav_and_server_headers.cache_clear()
+        responses.add(responses.OPTIONS, "http://s3.test/", status=200)
+
+        body = b"0123456789abcdef"
+        url = "http://s3.test/big.dat?AWSAccessKeyId=key&Signature=sig&Expires=1000"
+
+        def serve_range(request):
+            byte_range = request.headers.get("Range")
+            if byte_range is None:
+                return (200, {"Accept-Ranges": "bytes"}, body)
+            start, end = re.fullmatch(r"bytes=(\d+)-(\d*)", byte_range).groups()
+            first = int(start)
+            last = int(end) if end else len(body) - 1
+            chunk = body[first : last + 1]
+            return (
+                206,
+                {
+                    "Accept-Ranges": "bytes",
+                    "Content-Range": f"bytes {first}-{first + len(chunk) - 1}/{len(body)}",
+                },
+                chunk,
+            )
+
+        responses.add_callback(responses.GET, url, callback=serve_range)
+
+        with ResourcePath(url).open("rb") as handle:
+            handle.seek(-4, io.SEEK_END)
+            self.assertEqual(handle.read(), b"cdef")
+
+        probes = [
+            call.request
+            for call in responses.calls
+            if call.request.method == "GET" and call.request.headers.get("Range") == "bytes=0-0"
+        ]
+        self.assertEqual(len(probes), 1)
+
 
 class HttpReadWriteWebdavTestCase(GenericReadWriteTestCase, unittest.TestCase):
     """Test with a real webDAV server, as opposed to mocking responses."""