From bf8e72e47bd59d8ce4ac7f4fde8a83a279b8bc2f Mon Sep 17 00:00:00 2001 From: Cycloctane Date: Sun, 12 Jan 2025 04:27:12 +0800 Subject: [PATCH 01/17] add last-modified support for static --- falcon/routing/static.py | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/falcon/routing/static.py b/falcon/routing/static.py index 57e327ab6..eb7903fb6 100644 --- a/falcon/routing/static.py +++ b/falcon/routing/static.py @@ -1,6 +1,7 @@ from __future__ import annotations import asyncio +from datetime import datetime, timezone from functools import partial import io import os @@ -18,7 +19,7 @@ def _open_range( - file_path: Union[str, Path], req_range: Optional[Tuple[int, int]] + fh: io.BufferedReader, st: os.stat_result, req_range: Optional[Tuple[int, int]] ) -> Tuple[ReadableIO, int, Optional[Tuple[int, int, int]]]: """Open a file for a ranged request. @@ -32,8 +33,7 @@ def _open_range( 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 + size = st.st_size if req_range is None: return fh, size, None @@ -217,23 +217,32 @@ def __call__(self, req: Request, resp: Response, **kw: Any) -> None: if '..' in file_path or not file_path.startswith(self._directory): raise falcon.HTTPNotFound() - req_range = req.range - if req.range_unit != 'bytes': - req_range = None try: - stream, length, content_range = _open_range(file_path, req_range) - resp.set_stream(stream, length) + fh = io.open(file_path, 'rb') + st = os.fstat(fh.fileno()) except IOError: if self._fallback_filename is None: raise falcon.HTTPNotFound() try: - stream, length, content_range = _open_range( - self._fallback_filename, req_range - ) - resp.set_stream(stream, length) - file_path = self._fallback_filename + fh = io.open(self._fallback_filename, 'rb') + st = os.fstat(fh.fileno()) except IOError: raise falcon.HTTPNotFound() + else: + file_path = self._fallback_filename + + resp.last_modified = datetime.fromtimestamp(st.st_mtime, timezone.utc) + if (req.if_modified_since is not None and + resp.last_modified <= req.if_modified_since): + resp.status = falcon.HTTP_304 + return + + req_range = req.range if req.range_unit == 'bytes' else None + try: + stream, length, content_range = _open_range(fh, st, req_range) + resp.set_stream(stream, length) + except IOError: + raise falcon.HTTPNotFound() suffix = os.path.splitext(file_path)[1] resp.content_type = resp.options.static_media_types.get( From 8b0992d4189892aaa7cb25c0b4e9a83b6cedaeb4 Mon Sep 17 00:00:00 2001 From: Cycloctane Date: Sun, 12 Jan 2025 04:31:48 +0800 Subject: [PATCH 02/17] add mtime attr to static test fakestat --- tests/test_static.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_static.py b/tests/test_static.py index 5830b904c..91a38981b 100644 --- a/tests/test_static.py +++ b/tests/test_static.py @@ -57,8 +57,9 @@ class FakeFD(int): pass class FakeStat: - def __init__(self, size): + def __init__(self, size, mtime): self.st_size = size + self.st_mtime = mtime if validate: validate(path) @@ -66,7 +67,7 @@ def __init__(self, size): data = path.encode() if content is None else content fake_file = io.BytesIO(data) fd = FakeFD(1337) - fd._stat = FakeStat(len(data)) + fd._stat = FakeStat(len(data), 1736617934) fake_file.fileno = lambda: fd patch.current_file = fake_file From 6165befabf31e685cc8152ad98997c6e2e3f3496 Mon Sep 17 00:00:00 2001 From: Cycloctane Date: Sun, 12 Jan 2025 04:47:32 +0800 Subject: [PATCH 03/17] update _open_range docstring --- falcon/routing/static.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/falcon/routing/static.py b/falcon/routing/static.py index eb7903fb6..fa497307d 100644 --- a/falcon/routing/static.py +++ b/falcon/routing/static.py @@ -24,7 +24,8 @@ def _open_range( """Open a file for a ranged request. Args: - file_path (str): Path to the file to open. + fh (io.BufferedReader): file handler of the file. + st (os.stat_result): fs stat result of the file. req_range (Optional[Tuple[int, int]]): Request.range value. Returns: tuple: Three-member tuple of (stream, content-length, content-range). From 613fce6f68a09bccc05138af0ca84fa1b1b54490 Mon Sep 17 00:00:00 2001 From: Cycloctane Date: Wed, 15 Jan 2025 00:08:32 +0800 Subject: [PATCH 04/17] close file handler before raising exception --- falcon/routing/static.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/falcon/routing/static.py b/falcon/routing/static.py index fa497307d..96a4d36e2 100644 --- a/falcon/routing/static.py +++ b/falcon/routing/static.py @@ -218,16 +218,21 @@ def __call__(self, req: Request, resp: Response, **kw: Any) -> None: if '..' in file_path or not file_path.startswith(self._directory): raise falcon.HTTPNotFound() + fh: Optional[io.BufferedReader] = None try: fh = io.open(file_path, 'rb') st = os.fstat(fh.fileno()) except IOError: if self._fallback_filename is None: + if fh is not None: + fh.close() raise falcon.HTTPNotFound() try: fh = io.open(self._fallback_filename, 'rb') st = os.fstat(fh.fileno()) except IOError: + if fh is not None: + fh.close() raise falcon.HTTPNotFound() else: file_path = self._fallback_filename @@ -243,6 +248,7 @@ def __call__(self, req: Request, resp: Response, **kw: Any) -> None: stream, length, content_range = _open_range(fh, st, req_range) resp.set_stream(stream, length) except IOError: + fh.close() raise falcon.HTTPNotFound() suffix = os.path.splitext(file_path)[1] From 4635a61ca0c6a9bc60b47e72ffb61fcd9a5d076f Mon Sep 17 00:00:00 2001 From: Cycloctane Date: Wed, 15 Jan 2025 02:50:40 +0800 Subject: [PATCH 05/17] fix `resp.last_modified` type --- falcon/routing/static.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/falcon/routing/static.py b/falcon/routing/static.py index 96a4d36e2..b786db8a0 100644 --- a/falcon/routing/static.py +++ b/falcon/routing/static.py @@ -237,9 +237,10 @@ def __call__(self, req: Request, resp: Response, **kw: Any) -> None: else: file_path = self._fallback_filename - resp.last_modified = datetime.fromtimestamp(st.st_mtime, timezone.utc) + last_modified = datetime.fromtimestamp(st.st_mtime, timezone.utc) + resp.last_modified = last_modified if (req.if_modified_since is not None and - resp.last_modified <= req.if_modified_since): + last_modified <= req.if_modified_since): resp.status = falcon.HTTP_304 return From cea552a597e1b124a37b2b3b6de1efc1ce3580db Mon Sep 17 00:00:00 2001 From: Cycloctane Date: Wed, 15 Jan 2025 04:45:30 +0800 Subject: [PATCH 06/17] add tests --- tests/test_static.py | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/tests/test_static.py b/tests/test_static.py index 91a38981b..b68e84dba 100644 --- a/tests/test_static.py +++ b/tests/test_static.py @@ -13,6 +13,9 @@ import falcon.testing as testing +MTIME = (1736617934, "Sat, 11 Jan 2025 17:52:14 GMT") + + def normalize_path(path): # NOTE(vytas): On CPython 3.13, ntpath.isabs() no longer returns True for # Unix-like absolute paths that start with a single \. @@ -67,7 +70,7 @@ def __init__(self, size, mtime): data = path.encode() if content is None else content fake_file = io.BytesIO(data) fd = FakeFD(1337) - fd._stat = FakeStat(len(data), 1736617934) + fd._stat = FakeStat(len(data), MTIME[0]) fake_file.fileno = lambda: fd patch.current_file = fake_file @@ -634,3 +637,33 @@ def test_options_request(client, patch_open): assert resp.text == '' assert int(resp.headers['Content-Length']) == 0 assert resp.headers['Access-Control-Allow-Methods'] == 'GET' + + +def test_last_modified(client, patch_open): + patch_open() + + client.app.add_static_route('/assets/', '/opt/somesite/assets') + + response = client.simulate_request(path='/assets/css/main.css') + assert response.status == falcon.HTTP_200 + assert response.headers['Last-Modified'] == MTIME[1] + + +def test_304_with_if_modified_since(client, patch_open): + patch_open() + + client.app.add_static_route('/assets/', '/opt/somesite/assets') + + resp = client.simulate_request( + path='/assets/css/main.css', + headers={"If-Modified-Since": "Sat, 11 Jan 2025 17:52:15 GMT"}, + ) + assert resp.status == falcon.HTTP_304 + assert resp.text == '' + + resp = client.simulate_request( + path='/assets/css/main.css', + headers={"If-Modified-Since": "Sat, 11 Jan 2025 17:52:13 GMT"}, + ) + assert resp.status == falcon.HTTP_200 + assert resp.text != '' From ccd7b5ebbbc5753c39ea3af98e40a3420aa80dc5 Mon Sep 17 00:00:00 2001 From: Cycloctane Date: Wed, 15 Jan 2025 20:21:00 +0800 Subject: [PATCH 07/17] replace `os.fstat` with `os.stat` --- falcon/routing/static.py | 23 ++++++++++------------- tests/test_static.py | 22 +++++++++++----------- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/falcon/routing/static.py b/falcon/routing/static.py index b786db8a0..d5e9b35a9 100644 --- a/falcon/routing/static.py +++ b/falcon/routing/static.py @@ -17,14 +17,16 @@ from falcon import Request from falcon import Response +_stat = os.stat + def _open_range( - fh: io.BufferedReader, st: os.stat_result, req_range: Optional[Tuple[int, int]] + file_path: Union[Path, str], st: os.stat_result, req_range: Optional[Tuple[int, int]] ) -> Tuple[ReadableIO, int, Optional[Tuple[int, int, int]]]: """Open a file for a ranged request. Args: - fh (io.BufferedReader): file handler of the file. + file_path (str): Path to the file to open. st (os.stat_result): fs stat result of the file. req_range (Optional[Tuple[int, int]]): Request.range value. Returns: @@ -34,6 +36,7 @@ def _open_range( possibly bounded, and the content-range will be a tuple of (start, end, size). """ + fh = io.open(file_path, 'rb') size = st.st_size if req_range is None: return fh, size, None @@ -218,21 +221,14 @@ def __call__(self, req: Request, resp: Response, **kw: Any) -> None: if '..' in file_path or not file_path.startswith(self._directory): raise falcon.HTTPNotFound() - fh: Optional[io.BufferedReader] = None try: - fh = io.open(file_path, 'rb') - st = os.fstat(fh.fileno()) + st = _stat(file_path) except IOError: if self._fallback_filename is None: - if fh is not None: - fh.close() raise falcon.HTTPNotFound() try: - fh = io.open(self._fallback_filename, 'rb') - st = os.fstat(fh.fileno()) + st = _stat(self._fallback_filename) except IOError: - if fh is not None: - fh.close() raise falcon.HTTPNotFound() else: file_path = self._fallback_filename @@ -246,10 +242,11 @@ def __call__(self, req: Request, resp: Response, **kw: Any) -> None: req_range = req.range if req.range_unit == 'bytes' else None try: - stream, length, content_range = _open_range(fh, st, req_range) + stream, length, content_range = _open_range(file_path, st, req_range) resp.set_stream(stream, length) + except PermissionError: + raise falcon.HTTPForbidden() except IOError: - fh.close() raise falcon.HTTPNotFound() suffix = os.path.splitext(file_path)[1] diff --git a/tests/test_static.py b/tests/test_static.py index b68e84dba..ab6aa446b 100644 --- a/tests/test_static.py +++ b/tests/test_static.py @@ -10,6 +10,7 @@ from falcon.routing import StaticRoute from falcon.routing import StaticRouteAsync from falcon.routing.static import _BoundedFile +import falcon.routing.static import falcon.testing as testing @@ -55,29 +56,28 @@ def create_sr(asgi, prefix, directory, **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, mtime): + self.st_size = size + self.st_mtime = mtime + self.st_mode = 0o100644 - class FakeStat: - def __init__(self, size, mtime): - self.st_size = size - self.st_mtime = mtime + def open(path, mode): 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), MTIME[0]) - fake_file.fileno = lambda: fd patch.current_file = fake_file return fake_file + def _stat(path, **kwargs): + return FakeStat(len(open(path, 'rb').getvalue()), 1736617934) + monkeypatch.setattr(io, 'open', open) - monkeypatch.setattr(os, 'fstat', lambda fileno: fileno._stat) + monkeypatch.setattr(falcon.routing.static, '_stat', _stat) patch.current_file = None return patch From cf105d52f5f03e399b7870fbd4a2d05a2e6db003 Mon Sep 17 00:00:00 2001 From: Cycloctane Date: Wed, 15 Jan 2025 21:49:50 +0800 Subject: [PATCH 08/17] add tests for read error --- tests/test_static.py | 49 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/tests/test_static.py b/tests/test_static.py index ab6aa446b..2df583a66 100644 --- a/tests/test_static.py +++ b/tests/test_static.py @@ -3,6 +3,7 @@ import os import pathlib import posixpath +from unittest import mock import pytest @@ -14,9 +15,6 @@ import falcon.testing as testing -MTIME = (1736617934, "Sat, 11 Jan 2025 17:52:14 GMT") - - def normalize_path(path): # NOTE(vytas): On CPython 3.13, ntpath.isabs() no longer returns True for # Unix-like absolute paths that start with a single \. @@ -55,7 +53,7 @@ def create_sr(asgi, prefix, directory, **kwargs): @pytest.fixture def patch_open(monkeypatch): - def patch(content=None, validate=None): + def patch(content=None, validate=None, mtime=1736617934): class FakeStat: def __init__(self, size, mtime): self.st_size = size @@ -73,11 +71,16 @@ def open(path, mode): patch.current_file = fake_file return fake_file - def _stat(path, **kwargs): - return FakeStat(len(open(path, 'rb').getvalue()), 1736617934) + def stat(path, **kwargs): + + if validate: + validate(path) + + data = path.encode() if content is None else content + return FakeStat(len(data), mtime) monkeypatch.setattr(io, 'open', open) - monkeypatch.setattr(falcon.routing.static, '_stat', _stat) + monkeypatch.setattr(falcon.routing.static, '_stat', stat) patch.current_file = None return patch @@ -640,17 +643,19 @@ def test_options_request(client, patch_open): def test_last_modified(client, patch_open): - patch_open() + mtime = (1736617934, "Sat, 11 Jan 2025 17:52:14 GMT") + patch_open(mtime=mtime[0]) client.app.add_static_route('/assets/', '/opt/somesite/assets') response = client.simulate_request(path='/assets/css/main.css') assert response.status == falcon.HTTP_200 - assert response.headers['Last-Modified'] == MTIME[1] + assert response.headers['Last-Modified'] == mtime[1] -def test_304_with_if_modified_since(client, patch_open): - patch_open() +def test_if_modified_since(client, patch_open): + mtime = (1736617934, "Sat, 11 Jan 2025 17:52:14 GMT") + patch_open(mtime=mtime[0]) client.app.add_static_route('/assets/', '/opt/somesite/assets') @@ -667,3 +672,25 @@ def test_304_with_if_modified_since(client, patch_open): ) assert resp.status == falcon.HTTP_200 assert resp.text != '' + + +def test_permission_error(client, patch_open): + patch_open() + client.app.add_static_route('/assets/', '/opt/somesite/assets') + + with mock.patch("io.open", mock.mock_open()) as m: + m.side_effect = PermissionError() + resp = client.simulate_request(path='/assets/css/main.css') + + assert resp.status == falcon.HTTP_403 + + +def test_read_error(client, patch_open): + patch_open() + client.app.add_static_route('/assets/', '/opt/somesite/assets') + + with mock.patch("io.open", mock.mock_open()) as m: + m.side_effect = IOError() + resp = client.simulate_request(path='/assets/css/main.css') + + assert resp.status == falcon.HTTP_404 From c5e53182b6b7401bdcc60217df426004be56d9e5 Mon Sep 17 00:00:00 2001 From: Cycloctane Date: Wed, 15 Jan 2025 22:12:35 +0800 Subject: [PATCH 09/17] remove useless st_mode in fakestat --- tests/test_static.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_static.py b/tests/test_static.py index 2df583a66..36f33e33c 100644 --- a/tests/test_static.py +++ b/tests/test_static.py @@ -58,7 +58,6 @@ class FakeStat: def __init__(self, size, mtime): self.st_size = size self.st_mtime = mtime - self.st_mode = 0o100644 def open(path, mode): From 8f9930c032b3288a2312fc7749db23a914bf43db Mon Sep 17 00:00:00 2001 From: Cycloctane Date: Wed, 15 Jan 2025 22:49:59 +0800 Subject: [PATCH 10/17] handle permission error for os.stat --- falcon/routing/static.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/falcon/routing/static.py b/falcon/routing/static.py index d5e9b35a9..b288d698d 100644 --- a/falcon/routing/static.py +++ b/falcon/routing/static.py @@ -223,11 +223,15 @@ def __call__(self, req: Request, resp: Response, **kw: Any) -> None: try: st = _stat(file_path) + except PermissionError: + raise falcon.HTTPForbidden() except IOError: if self._fallback_filename is None: raise falcon.HTTPNotFound() try: st = _stat(self._fallback_filename) + except PermissionError: + raise falcon.HTTPForbidden() except IOError: raise falcon.HTTPNotFound() else: From 7c794f2a2395a3a831cf37f4ed582c7d566e12c9 Mon Sep 17 00:00:00 2001 From: Cycloctane Date: Wed, 15 Jan 2025 22:50:24 +0800 Subject: [PATCH 11/17] add more tests for permission error --- tests/test_static.py | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/tests/test_static.py b/tests/test_static.py index 36f33e33c..0569bfc1a 100644 --- a/tests/test_static.py +++ b/tests/test_static.py @@ -673,7 +673,30 @@ def test_if_modified_since(client, patch_open): assert resp.text != '' -def test_permission_error(client, patch_open): +@pytest.mark.parametrize('use_fallback', [True, False]) +def test_permission_error( + client, + patch_open, + use_fallback, + monkeypatch +): + def validate(path): + if use_fallback and not path.endswith('fallback.css'): + raise IOError() + raise PermissionError() + + patch_open(validate=validate) + monkeypatch.setattr('os.path.isfile', lambda file: file.endswith('fallback.css')) + + client.app.add_static_route( + '/assets/', '/opt/somesite/assets', fallback_filename='fallback.css' + ) + resp = client.simulate_request(path='/assets/css/main.css') + + assert resp.status == falcon.HTTP_403 + + +def test_read_permission_error(client, patch_open): patch_open() client.app.add_static_route('/assets/', '/opt/somesite/assets') @@ -684,7 +707,7 @@ def test_permission_error(client, patch_open): assert resp.status == falcon.HTTP_403 -def test_read_error(client, patch_open): +def test_read_ioerror(client, patch_open): patch_open() client.app.add_static_route('/assets/', '/opt/somesite/assets') From 6b7dd2158e31fbc7319213ee2258fbe6ea7c9479 Mon Sep 17 00:00:00 2001 From: Cycloctane Date: Wed, 15 Jan 2025 23:15:41 +0800 Subject: [PATCH 12/17] format --- falcon/routing/static.py | 14 +++++++++----- tests/test_static.py | 6 ++++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/falcon/routing/static.py b/falcon/routing/static.py index b288d698d..153cb0eca 100644 --- a/falcon/routing/static.py +++ b/falcon/routing/static.py @@ -1,7 +1,8 @@ from __future__ import annotations import asyncio -from datetime import datetime, timezone +from datetime import datetime +from datetime import timezone from functools import partial import io import os @@ -21,7 +22,9 @@ def _open_range( - file_path: Union[Path, str], st: os.stat_result, req_range: Optional[Tuple[int, int]] + file_path: Union[Path, str], + st: os.stat_result, + req_range: Optional[Tuple[int, int]] ) -> Tuple[ReadableIO, int, Optional[Tuple[int, int, int]]]: """Open a file for a ranged request. @@ -230,12 +233,11 @@ def __call__(self, req: Request, resp: Response, **kw: Any) -> None: raise falcon.HTTPNotFound() try: st = _stat(self._fallback_filename) + file_path = self._fallback_filename except PermissionError: raise falcon.HTTPForbidden() except IOError: raise falcon.HTTPNotFound() - else: - file_path = self._fallback_filename last_modified = datetime.fromtimestamp(st.st_mtime, timezone.utc) resp.last_modified = last_modified @@ -246,7 +248,9 @@ def __call__(self, req: Request, resp: Response, **kw: Any) -> None: req_range = req.range if req.range_unit == 'bytes' else None try: - stream, length, content_range = _open_range(file_path, st, req_range) + stream, length, content_range = _open_range( + file_path, st, req_range + ) resp.set_stream(stream, length) except PermissionError: raise falcon.HTTPForbidden() diff --git a/tests/test_static.py b/tests/test_static.py index 0569bfc1a..11b4297ed 100644 --- a/tests/test_static.py +++ b/tests/test_static.py @@ -10,8 +10,8 @@ import falcon from falcon.routing import StaticRoute from falcon.routing import StaticRouteAsync -from falcon.routing.static import _BoundedFile import falcon.routing.static +from falcon.routing.static import _BoundedFile import falcon.testing as testing @@ -686,7 +686,9 @@ def validate(path): raise PermissionError() patch_open(validate=validate) - monkeypatch.setattr('os.path.isfile', lambda file: file.endswith('fallback.css')) + monkeypatch.setattr( + 'os.path.isfile', lambda file: file.endswith('fallback.css') + ) client.app.add_static_route( '/assets/', '/opt/somesite/assets', fallback_filename='fallback.css' From 673db30137bf280ed255dad843a2fb70859136e2 Mon Sep 17 00:00:00 2001 From: Cycloctane Date: Thu, 16 Jan 2025 04:00:46 +0800 Subject: [PATCH 13/17] revert "replace `os.fstat` with `os.stat`" --- falcon/routing/static.py | 56 ++++++++++++++++++++++------------------ tests/test_static.py | 53 ++++++++++++++++--------------------- 2 files changed, 53 insertions(+), 56 deletions(-) diff --git a/falcon/routing/static.py b/falcon/routing/static.py index 153cb0eca..8cd0f4e8b 100644 --- a/falcon/routing/static.py +++ b/falcon/routing/static.py @@ -18,18 +18,34 @@ from falcon import Request from falcon import Response -_stat = os.stat - -def _open_range( - file_path: Union[Path, str], +def _open_file( + file_path: Union[str, Path] +) -> Tuple[io.BufferedReader, os.stat_result]: + fh: Optional[io.BufferedReader] = None + try: + fh = io.open(file_path, 'rb') + st = os.fstat(fh.fileno()) + except PermissionError: + if fh is not None: + fh.close() + raise falcon.HTTPForbidden() + except IOError: + if fh is not None: + fh.close() + raise falcon.HTTPNotFound() + return fh, st + + +def _set_range( + fh: io.BufferedReader, st: os.stat_result, req_range: Optional[Tuple[int, int]] ) -> Tuple[ReadableIO, int, Optional[Tuple[int, int, int]]]: - """Open a file for a ranged request. + """Process file handle for a ranged request. Args: - file_path (str): Path to the file to open. + fh (io.BufferedReader): file handle of the file. st (os.stat_result): fs stat result of the file. req_range (Optional[Tuple[int, int]]): Request.range value. Returns: @@ -39,7 +55,6 @@ def _open_range( possibly bounded, and the content-range will be a tuple of (start, end, size). """ - fh = io.open(file_path, 'rb') size = st.st_size if req_range is None: return fh, size, None @@ -224,20 +239,14 @@ def __call__(self, req: Request, resp: Response, **kw: Any) -> None: if '..' in file_path or not file_path.startswith(self._directory): raise falcon.HTTPNotFound() - try: - st = _stat(file_path) - except PermissionError: - raise falcon.HTTPForbidden() - except IOError: - if self._fallback_filename is None: - raise falcon.HTTPNotFound() + if self._fallback_filename is None: + fh, st = _open_file(file_path) + else: try: - st = _stat(self._fallback_filename) + fh, st = _open_file(file_path) + except falcon.HTTPNotFound: + fh, st = _open_file(self._fallback_filename) file_path = self._fallback_filename - except PermissionError: - raise falcon.HTTPForbidden() - except IOError: - raise falcon.HTTPNotFound() last_modified = datetime.fromtimestamp(st.st_mtime, timezone.utc) resp.last_modified = last_modified @@ -248,15 +257,12 @@ def __call__(self, req: Request, resp: Response, **kw: Any) -> None: req_range = req.range if req.range_unit == 'bytes' else None try: - stream, length, content_range = _open_range( - file_path, st, req_range - ) - resp.set_stream(stream, length) - except PermissionError: - raise falcon.HTTPForbidden() + stream, length, content_range = _set_range(fh, st, req_range) except IOError: + fh.close() raise falcon.HTTPNotFound() + resp.set_stream(stream, length) suffix = os.path.splitext(file_path)[1] resp.content_type = resp.options.static_media_types.get( suffix, 'application/octet-stream' diff --git a/tests/test_static.py b/tests/test_static.py index 11b4297ed..2a76b8ec0 100644 --- a/tests/test_static.py +++ b/tests/test_static.py @@ -3,7 +3,6 @@ import os import pathlib import posixpath -from unittest import mock import pytest @@ -54,32 +53,29 @@ def create_sr(asgi, prefix, directory, **kwargs): @pytest.fixture def patch_open(monkeypatch): def patch(content=None, validate=None, mtime=1736617934): - class FakeStat: - def __init__(self, size, mtime): - self.st_size = size - self.st_mtime = mtime - def open(path, mode): + class FakeFD(int): + pass + + class FakeStat: + def __init__(self, size): + self.st_size = size + self.st_mtime = mtime 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 patch.current_file = fake_file return fake_file - def stat(path, **kwargs): - - if validate: - validate(path) - - data = path.encode() if content is None else content - return FakeStat(len(data), mtime) - monkeypatch.setattr(io, 'open', open) - monkeypatch.setattr(falcon.routing.static, '_stat', stat) + monkeypatch.setattr(os, 'fstat', lambda fileno: fileno._stat) patch.current_file = None return patch @@ -698,23 +694,18 @@ def validate(path): assert resp.status == falcon.HTTP_403 -def test_read_permission_error(client, patch_open): - patch_open() - client.app.add_static_route('/assets/', '/opt/somesite/assets') - - with mock.patch("io.open", mock.mock_open()) as m: - m.side_effect = PermissionError() - resp = client.simulate_request(path='/assets/css/main.css') - - assert resp.status == falcon.HTTP_403 - +def test_ioerror(client, patch_open, monkeypatch): + def validate(path): + raise IOError() -def test_read_ioerror(client, patch_open): - patch_open() - client.app.add_static_route('/assets/', '/opt/somesite/assets') + patch_open(validate=validate) + monkeypatch.setattr( + 'os.path.isfile', lambda file: file.endswith('fallback.css') + ) - with mock.patch("io.open", mock.mock_open()) as m: - m.side_effect = IOError() - resp = client.simulate_request(path='/assets/css/main.css') + client.app.add_static_route( + '/assets/', '/opt/somesite/assets', fallback_filename='fallback.css' + ) + resp = client.simulate_request(path='/assets/css/main.css') assert resp.status == falcon.HTTP_404 From c728e8f2799527b53a529b4c3af94be6881724bc Mon Sep 17 00:00:00 2001 From: Cycloctane Date: Thu, 16 Jan 2025 04:38:27 +0800 Subject: [PATCH 14/17] add test for coverage --- tests/test_static.py | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/tests/test_static.py b/tests/test_static.py index 2a76b8ec0..724d49421 100644 --- a/tests/test_static.py +++ b/tests/test_static.py @@ -3,13 +3,13 @@ import os import pathlib import posixpath +from unittest import mock import pytest import falcon from falcon.routing import StaticRoute from falcon.routing import StaticRouteAsync -import falcon.routing.static from falcon.routing.static import _BoundedFile import falcon.testing as testing @@ -709,3 +709,36 @@ def validate(path): resp = client.simulate_request(path='/assets/css/main.css') assert resp.status == falcon.HTTP_404 + + +@pytest.mark.parametrize('error_type', [PermissionError, FileNotFoundError]) +def test_fstat_error(client, patch_open, error_type): + patch_open() + + client.app.add_static_route('/assets/', '/opt/somesite/assets') + + with mock.patch("os.fstat") as m: + m.side_effect = error_type() + resp = client.simulate_request(path='/assets/css/main.css') + + if isinstance(error_type(), PermissionError): + assert resp.status == falcon.HTTP_403 + else: + assert resp.status == falcon.HTTP_404 + + assert patch_open.current_file is not None + assert patch_open.current_file.closed + + +def test_set_range_error(client, patch_open): + patch_open() + + client.app.add_static_route('/assets/', '/opt/somesite/assets') + + with mock.patch("falcon.routing.static._set_range") as m: + m.side_effect = IOError() + resp = client.simulate_request(path='/assets/css/main.css') + + assert resp.status == falcon.HTTP_404 + assert patch_open.current_file is not None + assert patch_open.current_file.closed From 5b48366052bdefa5fc5a1d51c89c5f859ba98d75 Mon Sep 17 00:00:00 2001 From: Cycloctane Date: Thu, 16 Jan 2025 04:44:52 +0800 Subject: [PATCH 15/17] update tests --- tests/test_static.py | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/tests/test_static.py b/tests/test_static.py index 724d49421..61db85420 100644 --- a/tests/test_static.py +++ b/tests/test_static.py @@ -694,23 +694,6 @@ def validate(path): assert resp.status == falcon.HTTP_403 -def test_ioerror(client, patch_open, monkeypatch): - def validate(path): - raise IOError() - - patch_open(validate=validate) - monkeypatch.setattr( - 'os.path.isfile', lambda file: file.endswith('fallback.css') - ) - - client.app.add_static_route( - '/assets/', '/opt/somesite/assets', fallback_filename='fallback.css' - ) - resp = client.simulate_request(path='/assets/css/main.css') - - assert resp.status == falcon.HTTP_404 - - @pytest.mark.parametrize('error_type', [PermissionError, FileNotFoundError]) def test_fstat_error(client, patch_open, error_type): patch_open() @@ -721,7 +704,7 @@ def test_fstat_error(client, patch_open, error_type): m.side_effect = error_type() resp = client.simulate_request(path='/assets/css/main.css') - if isinstance(error_type(), PermissionError): + if error_type == PermissionError: assert resp.status == falcon.HTTP_403 else: assert resp.status == falcon.HTTP_404 From 4885c8b1b76b4003731c043a1af79a72b55ebfbe Mon Sep 17 00:00:00 2001 From: Cycloctane Date: Fri, 17 Jan 2025 19:42:31 +0800 Subject: [PATCH 16/17] fix pep8 warning --- tests/test_static.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_static.py b/tests/test_static.py index 61db85420..97fc6c7db 100644 --- a/tests/test_static.py +++ b/tests/test_static.py @@ -704,7 +704,7 @@ def test_fstat_error(client, patch_open, error_type): m.side_effect = error_type() resp = client.simulate_request(path='/assets/css/main.css') - if error_type == PermissionError: + if error_type is PermissionError: assert resp.status == falcon.HTTP_403 else: assert resp.status == falcon.HTTP_404 From c3f1d58575619d262b38a87445c36a5a21da753c Mon Sep 17 00:00:00 2001 From: Cycloctane Date: Tue, 21 Jan 2025 03:10:12 +0800 Subject: [PATCH 17/17] Format with `ruff` --- falcon/routing/static.py | 11 +++-------- tests/test_static.py | 23 ++++++++--------------- 2 files changed, 11 insertions(+), 23 deletions(-) diff --git a/falcon/routing/static.py b/falcon/routing/static.py index 8cd0f4e8b..6c0685b9c 100644 --- a/falcon/routing/static.py +++ b/falcon/routing/static.py @@ -19,9 +19,7 @@ from falcon import Response -def _open_file( - file_path: Union[str, Path] -) -> Tuple[io.BufferedReader, os.stat_result]: +def _open_file(file_path: Union[str, Path]) -> Tuple[io.BufferedReader, os.stat_result]: fh: Optional[io.BufferedReader] = None try: fh = io.open(file_path, 'rb') @@ -38,9 +36,7 @@ def _open_file( def _set_range( - fh: io.BufferedReader, - st: os.stat_result, - req_range: Optional[Tuple[int, int]] + fh: io.BufferedReader, st: os.stat_result, req_range: Optional[Tuple[int, int]] ) -> Tuple[ReadableIO, int, Optional[Tuple[int, int, int]]]: """Process file handle for a ranged request. @@ -250,8 +246,7 @@ def __call__(self, req: Request, resp: Response, **kw: Any) -> None: last_modified = datetime.fromtimestamp(st.st_mtime, timezone.utc) resp.last_modified = last_modified - if (req.if_modified_since is not None and - last_modified <= req.if_modified_since): + if req.if_modified_since is not None and last_modified <= req.if_modified_since: resp.status = falcon.HTTP_304 return diff --git a/tests/test_static.py b/tests/test_static.py index 97fc6c7db..20a9a6d51 100644 --- a/tests/test_static.py +++ b/tests/test_static.py @@ -638,7 +638,7 @@ def test_options_request(client, patch_open): def test_last_modified(client, patch_open): - mtime = (1736617934, "Sat, 11 Jan 2025 17:52:14 GMT") + mtime = (1736617934, 'Sat, 11 Jan 2025 17:52:14 GMT') patch_open(mtime=mtime[0]) client.app.add_static_route('/assets/', '/opt/somesite/assets') @@ -649,42 +649,35 @@ def test_last_modified(client, patch_open): def test_if_modified_since(client, patch_open): - mtime = (1736617934, "Sat, 11 Jan 2025 17:52:14 GMT") + mtime = (1736617934, 'Sat, 11 Jan 2025 17:52:14 GMT') patch_open(mtime=mtime[0]) client.app.add_static_route('/assets/', '/opt/somesite/assets') resp = client.simulate_request( path='/assets/css/main.css', - headers={"If-Modified-Since": "Sat, 11 Jan 2025 17:52:15 GMT"}, + headers={'If-Modified-Since': 'Sat, 11 Jan 2025 17:52:15 GMT'}, ) assert resp.status == falcon.HTTP_304 assert resp.text == '' resp = client.simulate_request( path='/assets/css/main.css', - headers={"If-Modified-Since": "Sat, 11 Jan 2025 17:52:13 GMT"}, + headers={'If-Modified-Since': 'Sat, 11 Jan 2025 17:52:13 GMT'}, ) assert resp.status == falcon.HTTP_200 assert resp.text != '' @pytest.mark.parametrize('use_fallback', [True, False]) -def test_permission_error( - client, - patch_open, - use_fallback, - monkeypatch -): +def test_permission_error(client, patch_open, use_fallback, monkeypatch): def validate(path): if use_fallback and not path.endswith('fallback.css'): raise IOError() raise PermissionError() patch_open(validate=validate) - monkeypatch.setattr( - 'os.path.isfile', lambda file: file.endswith('fallback.css') - ) + monkeypatch.setattr('os.path.isfile', lambda file: file.endswith('fallback.css')) client.app.add_static_route( '/assets/', '/opt/somesite/assets', fallback_filename='fallback.css' @@ -700,7 +693,7 @@ def test_fstat_error(client, patch_open, error_type): client.app.add_static_route('/assets/', '/opt/somesite/assets') - with mock.patch("os.fstat") as m: + with mock.patch('os.fstat') as m: m.side_effect = error_type() resp = client.simulate_request(path='/assets/css/main.css') @@ -718,7 +711,7 @@ def test_set_range_error(client, patch_open): client.app.add_static_route('/assets/', '/opt/somesite/assets') - with mock.patch("falcon.routing.static._set_range") as m: + with mock.patch('falcon.routing.static._set_range') as m: m.side_effect = IOError() resp = client.simulate_request(path='/assets/css/main.css')