From 69739f5f9bf28d6e35c8ceacafad4554056fabd5 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@pythonspeed.com>
Date: Wed, 20 Jul 2022 11:42:01 -0400
Subject: [PATCH] Handle case where requested range results in empty response.

---
 docs/proposed/http-storage-node-protocol.rst |  2 ++
 src/allmydata/storage/http_client.py         |  6 +++-
 src/allmydata/storage/http_server.py         | 29 +++++++++-----------
 3 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/docs/proposed/http-storage-node-protocol.rst b/docs/proposed/http-storage-node-protocol.rst
index 7e0b4a542..6a4e4136a 100644
--- a/docs/proposed/http-storage-node-protocol.rst
+++ b/docs/proposed/http-storage-node-protocol.rst
@@ -654,6 +654,8 @@ The ``Range`` header may be used to request exactly one ``bytes`` range, in whic
 Interpretation and response behavior is as specified in RFC 7233 § 4.1.
 Multiple ranges in a single request are *not* supported; open-ended ranges are also not supported.
 
+If the response to a query is an empty range, the ``NO CONTENT`` (204) response code will be used.
+
 Discussion
 ``````````
 
diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py
index 11c9ab2fc..236ec970f 100644
--- a/src/allmydata/storage/http_client.py
+++ b/src/allmydata/storage/http_client.py
@@ -468,6 +468,10 @@ def read_share_chunk(
             {"range": [Range("bytes", [(offset, offset + length)]).to_header()]}
         ),
     )
+
+    if response.code == http.NO_CONTENT:
+        return b""
+
     if response.code == http.PARTIAL_CONTENT:
         content_range = parse_content_range_header(
             response.headers.getRawHeaders("content-range")[0]
@@ -488,7 +492,7 @@ def read_share_chunk(
                 + f"didn't match Content-Range header ({supposed_length})"
             )
         body.seek(0)
-        returnValue(body.read())
+        return body.read()
     else:
         # Technically HTTP allows sending an OK with full body under these
         # circumstances, but the server is not designed to do that so we ignore
diff --git a/src/allmydata/storage/http_server.py b/src/allmydata/storage/http_server.py
index 82d3d4794..cb55afffe 100644
--- a/src/allmydata/storage/http_server.py
+++ b/src/allmydata/storage/http_server.py
@@ -343,27 +343,12 @@ class _ReadRangeProducer:
     result: Deferred
     start: int
     remaining: int
-    first_read: bool = field(default=True)
 
     def resumeProducing(self):
         to_read = min(self.remaining, 65536)
         data = self.read_data(self.start, to_read)
         assert len(data) <= to_read
 
-        if self.first_read and self.remaining > 0:
-            # For empty bodies the content-range header makes no sense since
-            # the end of the range is inclusive. Actual conversion from
-            # Python's exclusive ranges to inclusive ranges is handled by
-            # werkzeug. The case where we're reading beyond the end of the
-            # share is handled by the caller, read_range().
-            self.request.setHeader(
-                "content-range",
-                ContentRange(
-                    "bytes", self.start, self.start + self.remaining
-                ).to_header(),
-            )
-            self.first_read = False
-
         if not data and self.remaining > 0:
             d, self.result = self.result, None
             d.errback(
@@ -448,9 +433,21 @@ def read_range(
     # If we're being ask to read beyond the length of the share, just read
     # less:
     end = min(end, share_length)
-    # TODO when if end is now <= offset?
+    if offset >= end:
+        # Basically we'd need to return an empty body. However, the
+        # Content-Range header can't actually represent empty lengths... so
+        # (mis)use 204 response code to indicate that.
+        raise _HTTPError(http.NO_CONTENT)
 
     request.setResponseCode(http.PARTIAL_CONTENT)
+
+    # Actual conversion from Python's exclusive ranges to inclusive ranges is
+    # handled by werkzeug.
+    request.setHeader(
+        "content-range",
+        ContentRange("bytes", offset, end).to_header(),
+    )
+
     d = Deferred()
     request.registerProducer(
         _ReadRangeProducer(