Fix: Requesting range off end of file does not return 416 status code

Close #281

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## 4.2.1 / Unreleased
 - Install pam_dc dependencies using extra syntax: `pip install wsgidav[pam]`
+- #281 Requesting range off end of file does not return 416 status code
 
 ## 4.2.0 / 2023-02-18
 

wsgidav/dav_error.py

@@ -182,14 +182,21 @@ class DAVError(Exception):
     #     This would be helpful for debugging.
 
     def __init__(
-        self, status_code, context_info=None, *, src_exception=None, err_condition=None
+        self,
+        status_code,
+        context_info=None,
+        *,
+        src_exception=None,
+        err_condition=None,
+        add_headers=None,
     ):
         # allow passing of Pre- and Postconditions, see
         # http://www.webdav.org/specs/rfc4918.html#precondition.postcondition.xml.elements
         self.value = int(status_code)
         self.context_info = context_info
         self.src_exception = src_exception
         self.err_condition = err_condition
+        self.add_headers = add_headers
         if util.is_str(err_condition):
             self.err_condition = DAVErrorCondition(err_condition)
         assert (
@@ -204,18 +211,21 @@ def get_user_info(self):
         if self.value in ERROR_DESCRIPTIONS:
             s = "{}".format(ERROR_DESCRIPTIONS[self.value])
         else:
-            s = "{}".format(self.value)
+            s = f"{self.value}"
 
         if self.context_info:
-            s += ": {}".format(self.context_info)
+            s += f": {self.context_info}"
         elif self.value in ERROR_RESPONSES:
             s += ": {}".format(ERROR_RESPONSES[self.value])
 
         if self.src_exception:
-            s += "\n    Source exception: {!r}".format(self.src_exception)
+            s += f"\n    Source exception: {self.src_exception!r}"
 
         if self.err_condition:
-            s += "\n    Error condition: {!r}".format(self.err_condition)
+            s += f"\n    Error condition: {self.err_condition!r}"
+
+        # if self.add_headers:
+        #     s += f"\n    Add headers: {self.add_headers}"
         return s
 
     def get_response_page(self):
@@ -257,8 +267,7 @@ def get_http_status_code(v):
     """Return HTTP response code as integer, e.g. 204."""
     if hasattr(v, "value"):
         return int(v.value)  # v is a DAVError
-    else:
-        return int(v)
+    return int(v)
 
 
 def get_http_status_string(v):

wsgidav/error_printer.py

@@ -105,15 +105,16 @@ def __call__(self, environ, start_response):
             # If exception has pre-/post-condition: return as XML response,
             # else return as HTML
             content_type, body = e.get_response_page()
-
+            headers = e.add_headers or []
             # TODO: provide exc_info=sys.exc_info()?
             start_response(
                 status,
                 [
                     ("Content-Type", content_type),
                     ("Content-Length", str(len(body))),
                     ("Date", util.get_rfc1123_time()),
-                ],
+                ]
+                + headers,
             )
             yield body
             return

wsgidav/request_server.py

@@ -1627,15 +1627,15 @@ def _send_resource(self, environ, start_response, is_head_method):
                 if etag is None or if_range != etag:
                     do_ignore_ranges = True
 
-        ispartialranges = False
+        is_partial_ranges = False
         if "HTTP_RANGE" in environ and not do_ignore_ranges:
-            ispartialranges = True
+            is_partial_ranges = True
             list_ranges, _totallength = util.obtain_content_ranges(
                 environ["HTTP_RANGE"], filesize
             )
             if len(list_ranges) == 0:
                 # No valid ranges present
-                self._fail(HTTP_RANGE_NOT_SATISFIABLE)
+                self._fail(HTTP_RANGE_NOT_SATISFIABLE, "No valid ranges present")
 
             # More than one range present -> take only the first range, since
             # multiple range returns require multipart, which is not supported
@@ -1665,19 +1665,17 @@ def _send_resource(self, environ, start_response, is_head_method):
             response_headers.append(("Accept-Ranges", "bytes"))
 
         if "response_headers" in environ["wsgidav.config"]:
-            customHeaders = environ["wsgidav.config"]["response_headers"]
-            for header, value in customHeaders:
+            custom_headers = environ["wsgidav.config"]["response_headers"]
+            for header, value in custom_headers:
                 response_headers.append((header, value))
 
         res.finalize_headers(environ, response_headers)
 
-        if ispartialranges:
-            # response_headers.append(("Content-Ranges", "bytes " + str(range_start) + "-" +
-            #    str(range_end) + "/" + str(range_length)))
+        if is_partial_ranges:
             response_headers.append(
                 (
                     "Content-Range",
-                    "bytes {}-{}/{}".format(range_start, range_end, filesize),
+                    f"bytes {range_start}-{range_end}/{filesize}",
                 )
             )
             start_response("206 Partial Content", response_headers)

wsgidav/util.py

@@ -31,6 +31,7 @@
     HTTP_NOT_MODIFIED,
     HTTP_OK,
     HTTP_PRECONDITION_FAILED,
+    HTTP_RANGE_NOT_SATISFIABLE,
     DAVError,
     as_DAVError,
     get_http_status_string,
@@ -928,7 +929,14 @@ def read_and_discard_input(environ):
             _logger.error("--> wsgi_input.read(): {}".format(sys.exc_info()))
 
 
-def fail(value, context_info=None, *, src_exception=None, err_condition=None):
+def fail(
+    value,
+    context_info=None,
+    *,
+    src_exception=None,
+    err_condition=None,
+    add_headers=None,
+):
     """Wrapper to raise (and log) DAVError."""
     if isinstance(value, Exception):
         e = as_DAVError(value)
@@ -938,6 +946,7 @@ def fail(value, context_info=None, *, src_exception=None, err_condition=None):
             context_info,
             src_exception=src_exception,
             err_condition=err_condition,
+            add_headers=add_headers,
         )
     _logger.debug("Raising DAVError {}".format(e.get_user_info()))
     raise e
@@ -1196,6 +1205,8 @@ def send_status_response(
     headers = []
     if add_headers:
         headers.extend(add_headers)
+    if isinstance(e, DAVError) and e.add_headers:
+        headers.extend(e.add_headers)
     #    if 'keep-alive' in environ.get('HTTP_CONNECTION', '').lower():
     #        headers += [
     #            ('Connection', 'keep-alive'),
@@ -1428,68 +1439,80 @@ def get_file_etag(file_path):
 # Range Specifiers
 reByteRangeSpecifier = re.compile("(([0-9]+)-([0-9]*))")
 reSuffixByteRangeSpecifier = re.compile("(-([0-9]+))")
-# reByteRangeSpecifier = re.compile("(([0-9]+)\-([0-9]*))")
-# reSuffixByteRangeSpecifier = re.compile("(\-([0-9]+))")
 
 
-def obtain_content_ranges(rangetext, filesize):
+def obtain_content_ranges(range_header, filesize):
     """
-    returns tuple (list, value)
+    See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Range
+
+    Return tuple (range_list, total_length)
 
-    list
+    range_list
         content ranges as values to their parsed components in the tuple
         (seek_position/abs position of first byte, abs position of last byte, num_of_bytes_to_read)
-    value
+    total_length
         total length for Content-Length
     """
-    listReturn = []
-    seqRanges = rangetext.split(",")
-    for subrange in seqRanges:
-        matched = False
-        if not matched:
-            mObj = reByteRangeSpecifier.search(subrange)
-            if mObj:
-                firstpos = int(mObj.group(2))
-                if mObj.group(3) == "":
-                    lastpos = filesize - 1
+    list_ranges = []
+    request_ranges = range_header.split(",")
+    for subrange in request_ranges:
+        is_matched = False
+        if not is_matched:
+            match = reByteRangeSpecifier.search(subrange)
+            if match:
+                range_start = int(match.group(2))
+
+                if range_start >= filesize:
+                    # TODO: set "Content-Range: bytes */filesize"
+                    fail(
+                        HTTP_RANGE_NOT_SATISFIABLE,
+                        f"Requested range starts behind file size ({range_start} >= {filesize})",
+                        add_headers=[("Content-Range", f"bytes */{filesize}")],
+                    )
+
+                if match.group(3) == "":
+                    # "START-"
+                    range_end = filesize - 1
                 else:
-                    lastpos = int(mObj.group(3))
-                if firstpos <= lastpos and firstpos < filesize:
-                    if lastpos >= filesize:
-                        lastpos = filesize - 1
-                    listReturn.append((firstpos, lastpos))
-                    matched = True
-        if not matched:
-            mObj = reSuffixByteRangeSpecifier.search(subrange)
-            if mObj:
-                firstpos = filesize - int(mObj.group(2))
-                if firstpos < 0:
-                    firstpos = 0
-                lastpos = filesize - 1
-                listReturn.append((firstpos, lastpos))
-
-                matched = True
+                    # "START-END"
+                    range_end = int(match.group(3))
+
+                if range_start <= range_end and range_start < filesize:
+                    if range_end >= filesize:
+                        range_end = filesize - 1
+                    list_ranges.append((range_start, range_end))
+                    is_matched = True
+
+        if not is_matched:
+            match = reSuffixByteRangeSpecifier.search(subrange)
+            if match:
+                range_start = filesize - int(match.group(2))
+                if range_start < 0:
+                    range_start = 0
+                range_end = filesize - 1
+                list_ranges.append((range_start, range_end))
+                is_matched = True
 
     # consolidate ranges
-    listReturn.sort()
-    listReturn2 = []
-    totallength = 0
-    while len(listReturn) > 0:
-        (rfirstpos, rlastpos) = listReturn.pop()
-        counter = len(listReturn)
+    list_ranges.sort()
+    list_ranges_2 = []
+    total_length = 0
+    while len(list_ranges) > 0:
+        (rfirstpos, rlastpos) = list_ranges.pop()
+        counter = len(list_ranges)
         while counter > 0:
-            (nfirstpos, nlastpos) = listReturn[counter - 1]
+            (nfirstpos, nlastpos) = list_ranges[counter - 1]
             if nlastpos < rfirstpos - 1 or nfirstpos > nlastpos + 1:
                 pass
             else:
                 rfirstpos = min(rfirstpos, nfirstpos)
                 rlastpos = max(rlastpos, nlastpos)
-                del listReturn[counter - 1]
+                del list_ranges[counter - 1]
             counter = counter - 1
-        listReturn2.append((rfirstpos, rlastpos, rlastpos - rfirstpos + 1))
-        totallength = totallength + rlastpos - rfirstpos + 1
+        list_ranges_2.append((rfirstpos, rlastpos, rlastpos - rfirstpos + 1))
+        total_length = total_length + rlastpos - rfirstpos + 1
 
-    return (listReturn2, totallength)
+    return (list_ranges_2, total_length)
 
 
 # ========================================================================