Implement Range header validation.

docs/proposed/http-storage-node-protocol.rst

@@ -644,7 +644,7 @@ Read a contiguous sequence of bytes from one share in one bucket.
 The response body is the raw share data (i.e., ``application/octet-stream``).
 The ``Range`` header may be used to request exactly one ``bytes`` range, in which case the response code will be 206 (partial content).
 Interpretation and response behavior is as specified in RFC 7233 § 4.1.
-Multiple ranges in a single request are *not* supported.
+Multiple ranges in a single request are *not* supported; open-ended ranges are also not supported.
 
 Discussion
 ``````````

src/allmydata/storage/http_server.py

@@ -291,16 +291,19 @@ class HTTPServer(object):
     )
     def read_share_chunk(self, request, authorization, storage_index, share_number):
         """Read a chunk for an already uploaded immutable."""
-        # TODO in https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3860
         # 2. missing range header should have response code 200 and return whole thing
-        # 3. malformed range header should result in error? or return everything?
-        # 4. non-bytes range results in error
-        # 5. ranges make sense semantically (positive, etc.)
-        # 6. multiple ranges fails with error
         # 7. missing end of range means "to the end of share"
         range_header = parse_range_header(request.getHeader("range"))
+        if (
+            range_header is None
+            or range_header.units != "bytes"
+            or len(range_header.ranges) > 1  # more than one range
+            or range_header.ranges[0][1] is None  # range without end
+        ):
+            request.setResponseCode(http.REQUESTED_RANGE_NOT_SATISFIABLE)
+            return b""
+
         offset, end = range_header.ranges[0]
-        assert end != None  # TODO support this case
 
         try:
             bucket = self._storage_server.get_buckets(storage_index)[share_number]

src/allmydata/test/test_storage_http.py

@@ -703,14 +703,38 @@ class ImmutableHTTPAPITests(SyncTestCase):
 
     def test_read_with_negative_offset_fails(self):
         """
-        The offset for reads cannot be negative.
+        Malformed or unsupported Range headers result in 416 (requested range
-
+        not satisfiable) error.
-        TBD in https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3860
         """
+        storage_index = self.upload(1)
 
-    def test_read_with_negative_length_fails(self):
+        def check_bad_range(bad_range_value):
-        """
+            client = StorageClientImmutables(
-        The length for reads cannot be negative.
+                StorageClientWithHeadersOverride(
+                    self.http.client, {"range": bad_range_value}
+                )
+            )
+
+            with self.assertRaises(ClientException) as e:
+                result_of(
+                    client.read_share_chunk(
+                        storage_index,
+                        1,
+                        0,
+                        10,
+                    )
+                )
+            self.assertEqual(e.exception.code, http.REQUESTED_RANGE_NOT_SATISFIABLE)
+
+        check_bad_range("molluscs=0-9")
+        check_bad_range("bytes=-2-9")
+        check_bad_range("bytes=0--10")
+        check_bad_range("bytes=-300-")
+        check_bad_range("bytes=")
+        # Multiple ranges are currently unsupported, even if they're
+        # semantically valid under HTTP:
+        check_bad_range("bytes=0-5, 6-7")
+        # Ranges without an end are currently unsupported, even if they're
+        # semantically valid under HTTP.
+        check_bad_range("bytes=0-")
 
-        TBD in https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3860
-        """