feat(static): set Content-Length for static file responses (#1991)

* test(WSGI servers): add a failing test for byterange support in the TDD fashion

* feat(static): set `Content-Length` by fstat()-ing open file

* fix(static): set the correct `Content-Length` for range responses

* docs(static): add a warning on app server timeout settings

* style: fighting black vs pep8 plugin quote styles

* test(static): improve tests as per review comments

* docs(newsfragments): add a newsfragment for the added feature.

docs/_newsfragments/1991.newandimproved.rst

@@ -0,0 +1,3 @@
+:func:`Static routes <falcon.App.add_static_route>` now set the
+``Content-Length`` header indicating a served file's size
+(or length of the rendered content range).

falcon/app.py

@@ -589,6 +589,13 @@ def add_static_route(
             For security reasons, the directory and the fallback_filename (if provided)
             should be read only for the account running the application.
 
+        Warning:
+            If you need to serve large files and/or progressive downloads (such
+            as in the case of video streaming) through the Falcon app, check
+            that your application server's timeout settings can accomodate the
+            expected request duration (for instance, the popular Gunicorn kills
+            ``sync`` workers after 30 seconds unless configured otherwise).
+
         Note:
             For ASGI apps, file reads are made non-blocking by scheduling
             them on the default executor.

falcon/routing/static.py

@@ -15,37 +15,48 @@ def _open_range(file_path, req_range):
         file_path (str): Path to the file to open.
         req_range (Optional[Tuple[int, int]]): Request.range value.
     Returns:
-        tuple: Two-member tuple of (stream, content-range). If req_range is
-            ``None`` or ignored, content-range will be ``None``; otherwise,
-            the stream will be appropriately seeked and possibly bounded,
-            and the content-range will be a tuple of (start, end, size).
+        tuple: Three-member tuple of (stream, content-length, content-range).
+            If req_range is ``None`` or ignored, content-range will be
+            ``None``; otherwise, the stream will be appropriately seeked and
+            possibly bounded, and the content-range will be a tuple of
+            (start, end, size).
     """
     fh = io.open(file_path, 'rb')
+    size = os.fstat(fh.fileno()).st_size
     if req_range is None:
-        return fh, None
+        return fh, size, None
 
     start, end = req_range
-    size = os.fstat(fh.fileno()).st_size
     if size == 0:
-        # Ignore Range headers for zero-byte files; just serve the empty body
-        # since Content-Range can't be used to express a zero-byte body
-        return fh, None
+        # NOTE(tipabu): Ignore Range headers for zero-byte files; just serve
+        #   the empty body since Content-Range can't be used to express a
+        #   zero-byte body.
+        return fh, 0, None
 
     if start < 0 and end == -1:
-        # Special case: only want the last N bytes
+        # NOTE(tipabu): Special case: only want the last N bytes.
         start = max(start, -size)
         fh.seek(start, os.SEEK_END)
-        return fh, (size + start, size - 1, size)
+        # NOTE(vytas): Wrap in order to prevent sendfile from being used, as
+        #   its implementation was found to be buggy in many popular WSGI
+        #   servers for open files with a non-zero offset.
+        return _BoundedFile(fh, -start), -start, (size + start, size - 1, size)
 
     if start >= size:
+        fh.close()
         raise falcon.HTTPRangeNotSatisfiable(size)
 
     fh.seek(start)
     if end == -1:
-        return fh, (start, size - 1, size)
+        # NOTE(vytas): Wrap in order to prevent sendfile from being used, as
+        #   its implementation was found to be buggy in many popular WSGI
+        #   servers for open files with a non-zero offset.
+        length = size - start
+        return _BoundedFile(fh, length), length, (start, size - 1, size)
 
     end = min(end, size - 1)
-    return _BoundedFile(fh, end - start + 1), (start, end, size)
+    length = end - start + 1
+    return _BoundedFile(fh, length), length, (start, end, size)
 
 
 class _BoundedFile:
@@ -184,14 +195,16 @@ def __call__(self, req, resp):
         if req.range_unit != 'bytes':
             req_range = None
         try:
-            resp.stream, content_range = _open_range(file_path, req_range)
+            stream, length, content_range = _open_range(file_path, req_range)
+            resp.set_stream(stream, length)
         except IOError:
             if self._fallback_filename is None:
                 raise falcon.HTTPNotFound()
             try:
-                resp.stream, content_range = _open_range(
+                stream, length, content_range = _open_range(
                     self._fallback_filename, req_range
                 )
+                resp.set_stream(stream, length)
                 file_path = self._fallback_filename
             except IOError:
                 raise falcon.HTTPNotFound()

tests/test_static.py

@@ -28,6 +28,33 @@ def create_sr(asgi, *args, **kwargs):
     return sr_type(*args, **kwargs)
 
 
+@pytest.fixture
+def patch_open(monkeypatch):
+    def patch(content=None, validate=None):
+        def open(path, mode):
+            class FakeFD(int):
+                pass
+
+            class FakeStat:
+                def __init__(self, size):
+                    self.st_size = size
+
+            if validate:
+                validate(path)
+
+            data = path.encode() if content is None else content
+            fake_file = io.BytesIO(data)
+            fd = FakeFD(1337)
+            fd._stat = FakeStat(len(data))
+            fake_file.fileno = lambda: fd
+            return fake_file
+
+        monkeypatch.setattr(io, 'open', open)
+        monkeypatch.setattr(os, 'fstat', lambda fileno: fileno._stat)
+
+    return patch
+
+
 @pytest.mark.parametrize(
     'uri',
     [
@@ -83,8 +110,8 @@ def create_sr(asgi, *args, **kwargs):
         '/static/\ufffdsomething',
     ],
 )
-def test_bad_path(asgi, uri, monkeypatch):
-    monkeypatch.setattr(io, 'open', lambda path, mode: io.BytesIO())
+def test_bad_path(asgi, uri, patch_open):
+    patch_open(b'')
 
     sr_type = StaticRouteAsync if asgi else StaticRoute
     sr = sr_type('/static', '/var/www/statics')
@@ -177,8 +204,8 @@ def test_invalid_args_fallback_filename(client, default):
         ),
     ],
 )
-def test_good_path(asgi, uri_prefix, uri_path, expected_path, mtype, monkeypatch):
-    monkeypatch.setattr(io, 'open', lambda path, mode: io.BytesIO(path.encode()))
+def test_good_path(asgi, uri_prefix, uri_path, expected_path, mtype, patch_open):
+    patch_open()
 
     sr = create_sr(asgi, uri_prefix, '/var/www/statics')
 
@@ -219,21 +246,20 @@ async def run():
 )
 @pytest.mark.parametrize('use_fallback', [True, False])
 def test_range_requests(
-    client, range_header, exp_content_range, exp_content, monkeypatch, use_fallback
+    client,
+    range_header,
+    exp_content_range,
+    exp_content,
+    patch_open,
+    monkeypatch,
+    use_fallback,
 ):
-    fake_file = io.BytesIO(b'0123456789abcdef')
-    fake_file.fileno = lambda: 123
-
-    def fake_open(path, mode):
+    def validate(path):
         if use_fallback and not path.endswith('index.html'):
             raise OSError(errno.ENOENT, 'File not found')
-        return fake_file
 
-    class FakeStat:
-        st_size = len(fake_file.getvalue())
+    patch_open(b'0123456789abcdef', validate=validate)
 
-    monkeypatch.setattr(io, 'open', fake_open)
-    monkeypatch.setattr(os, 'fstat', lambda fileno: FakeStat())
     monkeypatch.setattr('os.path.isfile', lambda file: file.endswith('index.html'))
 
     client.app.add_static_route(
@@ -248,6 +274,7 @@ class FakeStat:
     else:
         assert response.status == falcon.HTTP_206
     assert response.text == exp_content
+    assert int(response.headers['Content-Length']) == len(exp_content)
     assert response.headers.get('Content-Range') == exp_content_range
     assert response.headers.get('Accept-Ranges') == 'bytes'
     if use_fallback:
@@ -270,15 +297,8 @@ class FakeStat:
         'bytes=-30',
     ],
 )
-def test_range_request_zero_length(client, range_header, monkeypatch):
-    fake_file = io.BytesIO(b'')
-    fake_file.fileno = lambda: 123
-
-    class FakeStat:
-        st_size = 0
-
-    monkeypatch.setattr(io, 'open', lambda path, mode: fake_file)
-    monkeypatch.setattr(os, 'fstat', lambda fileno: FakeStat())
+def test_range_request_zero_length(client, range_header, patch_open):
+    patch_open(b'')
 
     client.app.add_static_route('/downloads', '/opt/somesite/downloads')
 
@@ -305,15 +325,8 @@ class FakeStat:
         ('bytes=16-', falcon.HTTP_416),
     ],
 )
-def test_bad_range_requests(client, range_header, exp_status, monkeypatch):
-    fake_file = io.BytesIO(b'0123456789abcdef')
-    fake_file.fileno = lambda: 123
-
-    class FakeStat:
-        st_size = len(fake_file.getvalue())
-
-    monkeypatch.setattr(io, 'open', lambda path, mode: fake_file)
-    monkeypatch.setattr(os, 'fstat', lambda fileno: FakeStat())
+def test_bad_range_requests(client, range_header, exp_status, patch_open):
+    patch_open(b'0123456789abcdef')
 
     client.app.add_static_route('/downloads', '/opt/somesite/downloads')
 
@@ -325,8 +338,8 @@ class FakeStat:
         assert response.headers.get('Content-Range') == 'bytes */16'
 
 
-def test_pathlib_path(asgi, monkeypatch):
-    monkeypatch.setattr(io, 'open', lambda path, mode: io.BytesIO(path.encode()))
+def test_pathlib_path(asgi, patch_open):
+    patch_open()
 
     sr = create_sr(asgi, '/static/', pathlib.Path('/var/www/statics'))
     req_path = '/static/css/test.css'
@@ -349,8 +362,8 @@ async def run():
     assert body.decode() == os.path.normpath('/var/www/statics/css/test.css')
 
 
-def test_lifo(client, monkeypatch):
-    monkeypatch.setattr(io, 'open', lambda path, mode: io.BytesIO(path.encode()))
+def test_lifo(client, patch_open):
+    patch_open()
 
     client.app.add_static_route('/downloads', '/opt/somesite/downloads')
     client.app.add_static_route('/downloads/archive', '/opt/somesite/x')
@@ -364,8 +377,8 @@ def test_lifo(client, monkeypatch):
     assert response.text == os.path.normpath('/opt/somesite/x/thingtoo.zip')
 
 
-def test_lifo_negative(client, monkeypatch):
-    monkeypatch.setattr(io, 'open', lambda path, mode: io.BytesIO(path.encode()))
+def test_lifo_negative(client, patch_open):
+    patch_open()
 
     client.app.add_static_route('/downloads/archive', '/opt/somesite/x')
     client.app.add_static_route('/downloads', '/opt/somesite/downloads')
@@ -381,8 +394,8 @@ def test_lifo_negative(client, monkeypatch):
     )
 
 
-def test_downloadable(client, monkeypatch):
-    monkeypatch.setattr(io, 'open', lambda path, mode: io.BytesIO(path.encode()))
+def test_downloadable(client, patch_open):
+    patch_open()
 
     client.app.add_static_route(
         '/downloads', '/opt/somesite/downloads', downloadable=True
@@ -433,15 +446,14 @@ def test_downloadable_not_found(client):
 )
 @pytest.mark.parametrize('downloadable', [True, False])
 def test_fallback_filename(
-    asgi, uri, default, expected, content_type, downloadable, monkeypatch
+    asgi, uri, default, expected, content_type, downloadable, patch_open, monkeypatch
 ):
-    def mock_open(path, mode):
-        if os.path.normpath(default) in path:
-            return io.BytesIO(path.encode())
+    def validate(path):
+        if os.path.normpath(default) not in path:
+            raise IOError()
 
-        raise IOError()
+    patch_open(validate=validate)
 
-    monkeypatch.setattr(io, 'open', mock_open)
     monkeypatch.setattr(
         'os.path.isfile', lambda file: os.path.normpath(default) in file
     )
@@ -494,14 +506,14 @@ async def run():
     ],
 )
 def test_e2e_fallback_filename(
-    client, monkeypatch, strip_slash, path, fallback, static_exp, assert_axp
+    client, patch_open, monkeypatch, strip_slash, path, fallback, static_exp, assert_axp
 ):
-    def mockOpen(path, mode):
-        if 'index' in path and 'raise' not in path:
-            return io.BytesIO(path.encode())
-        raise IOError()
+    def validate(path):
+        if 'index' not in path or 'raise' in path:
+            raise IOError()
+
+    patch_open(validate=validate)
 
-    monkeypatch.setattr(io, 'open', mockOpen)
     monkeypatch.setattr('os.path.isfile', lambda file: 'index' in file)
 
     client.app.req_options.strip_url_path_trailing_slash = strip_slash
@@ -517,6 +529,7 @@ def test(prefix, directory, expected):
         else:
             assert response.status == falcon.HTTP_200
             assert response.text == os.path.normpath(directory + expected)
+            assert int(response.headers['Content-Length']) == len(response.text)
 
     test('/static', '/opt/somesite/static/', static_exp)
     test('/assets', '/opt/somesite/assets/', assert_axp)