diff --git a/docs/_newsfragments/2337.bugfix.rst b/docs/_newsfragments/2337.bugfix.rst new file mode 100644 index 000000000..a4254909d --- /dev/null +++ b/docs/_newsfragments/2337.bugfix.rst @@ -0,0 +1,15 @@ +Static routes (as configured via +:meth:`~falcon.App.add_static_route`) did not implement proper ``HEAD`` +support: a ``HEAD`` request would needlessly open the underlying file (and +never close it, since the body was later discarded), even though a ``HEAD`` +response never includes a body. ``HEAD`` requests to static routes are now +served directly from the file's metadata (``stat``), without opening a file +handle, while still returning the same headers (including ``Content-Length``, +``ETag``, ``Last-Modified``, and ``Content-Range`` for ranged requests) that +the equivalent ``GET`` request would have produced. + +In addition, static routes now reject methods other than ``GET``, ``HEAD``, +and ``OPTIONS`` with a ``405 Method Not Allowed`` response (rather than +serving the file's contents for any method), and the ``Allow``/ +``Access-Control-Allow-Methods`` header set for ``OPTIONS`` requests now +correctly advertises ``GET, HEAD`` instead of just ``GET``. diff --git a/falcon/routing/static.py b/falcon/routing/static.py index a336026e9..70959cce0 100644 --- a/falcon/routing/static.py +++ b/falcon/routing/static.py @@ -39,6 +39,24 @@ def _open_file(file_path: str | Path) -> tuple[io.BufferedReader, os.stat_result return fh, st +def _stat_file(file_path: str | Path) -> os.stat_result: + """Stat a file for a static HEAD request, without opening a file handle. + + A HEAD response never includes a body, so there is no need to actually + open (and later have to remember to close) the underlying file; we only + need its metadata in order to set the same headers a GET would. + + Args: + file_path (Union[str, Path]): Path to the file to stat. + Returns: + os.stat_result: stat result for the file. + """ + try: + return os.stat(file_path) + except OSError: + raise falcon.HTTPNotFound() + + def _set_range( fh: io.BufferedReader, st: os.stat_result, req_range: tuple[int, int] | None ) -> tuple[ReadableIO, int, tuple[int, int, int] | None]: @@ -92,6 +110,48 @@ def _set_range( return _BoundedFile(fh, length), length, (start, end, size) +def _compute_head_range( + size: int, req_range: tuple[int, int] | None +) -> tuple[int, tuple[int, int, int] | None]: + """Compute (content-length, content-range) for a ranged HEAD request. + + This mirrors the offset/length arithmetic in :func:`_set_range`, but + does not require (and must not touch) an open file handle, since a HEAD + response never includes a body. + + Args: + size (int): Size in bytes of the requested file. + req_range (Optional[Tuple[int, int]]): Request.range value. + Returns: + tuple: Two-member tuple of (content-length, content-range), where + content-range is a tuple of (start, end, size), or ``None`` if + req_range is ``None`` or ignored. + """ + if req_range is None: + return size, None + + start, end = req_range + if size == 0: + # NOTE: Ignore Range headers for zero-byte files; see also the + # corresponding note in _set_range(). + return 0, None + + if start < 0 and end == -1: + start = max(start, -size) + return -start, (size + start, size - 1, size) + + if start >= size: + raise falcon.HTTPRangeNotSatisfiable(size) + + if end == -1: + length = size - start + return length, (start, size - 1, size) + + end = min(end, size - 1) + length = end - start + 1 + return length, (start, end, size) + + def _is_not_modified( req: falcon.Request, current_etag: str, last_modified: datetime ) -> bool: @@ -223,15 +283,50 @@ def match(self, path: str) -> bool: return path.startswith(self._prefix) return path.startswith(self._prefix) or path == self._prefix[:-1] + def _locate( + self, file_path: str, is_head: bool + ) -> tuple[os.stat_result, io.BufferedReader | None, str]: + """Resolve the file to serve for this request, honoring fallback_filename. + + Returns a three-member tuple of (stat result, file handle, file + path actually resolved). The file handle will be ``None`` for HEAD + requests, since those never require an open file handle (a HEAD + response never includes a body). + """ + if is_head: + if self._fallback_filename is None: + return _stat_file(file_path), None, file_path + try: + return _stat_file(file_path), None, file_path + except falcon.HTTPNotFound: + return ( + _stat_file(self._fallback_filename), + None, + self._fallback_filename, + ) + + if self._fallback_filename is None: + fh, st = _open_file(file_path) + return st, fh, file_path + try: + fh, st = _open_file(file_path) + return st, fh, file_path + except falcon.HTTPNotFound: + fh, st = _open_file(self._fallback_filename) + return st, fh, self._fallback_filename + def __call__(self, req: Request, resp: Response, **kw: Any) -> None: """Resource responder for this route.""" assert not kw if req.method == 'OPTIONS': # it's likely a CORS request. Set the allow header to the appropriate value. - resp.set_header('Allow', 'GET') + resp.set_header('Allow', 'GET, HEAD') resp.set_header('Content-Length', '0') return + if req.method not in ('GET', 'HEAD'): + raise falcon.HTTPMethodNotAllowed(['GET', 'HEAD']) + without_prefix = req.path[len(self._prefix) :] # NOTE(kgriffs): Check surrounding whitespace and strip trailing @@ -260,14 +355,8 @@ def __call__(self, req: Request, resp: Response, **kw: Any) -> None: if '..' in file_path or not file_path.startswith(self._directory): raise falcon.HTTPNotFound() - if self._fallback_filename is None: - fh, st = _open_file(file_path) - else: - try: - fh, st = _open_file(file_path) - except falcon.HTTPNotFound: - fh, st = _open_file(self._fallback_filename) - file_path = self._fallback_filename + is_head = req.method == 'HEAD' + st, fh, file_path = self._locate(file_path, is_head) etag = f'{int(st.st_mtime):x}-{st.st_size:x}' resp.etag = etag @@ -280,18 +369,26 @@ def __call__(self, req: Request, resp: Response, **kw: Any) -> None: resp.last_modified = last_modified if _is_not_modified(req, etag, last_modified): - fh.close() + if fh is not None: + fh.close() resp.status = falcon.HTTP_304 return req_range = req.range if req.range_unit == 'bytes' else None - try: - stream, length, content_range = _set_range(fh, st, req_range) - except OSError: - fh.close() - raise falcon.HTTPNotFound() - resp.set_stream(stream, length) + if is_head: + length, content_range = _compute_head_range(st.st_size, req_range) + resp.content_length = length + else: + assert fh is not None + try: + stream, length, content_range = _set_range(fh, st, req_range) + except OSError: + 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' @@ -319,7 +416,7 @@ async def __call__( # type: ignore[override] raise falcon.HTTPBadRequest() super().__call__(req, resp, **kw) - if resp.stream is not None: # None when in an option request + if resp.stream is not None: # None for OPTIONS, HEAD, and 304 responses # NOTE(kgriffs): Fixup resp.stream so that it is non-blocking resp.stream = _AsyncFileReader(resp.stream) # type: ignore[assignment,arg-type] diff --git a/tests/test_cors_middleware.py b/tests/test_cors_middleware.py index df1a0d589..3e2102599 100644 --- a/tests/test_cors_middleware.py +++ b/tests/test_cors_middleware.py @@ -127,7 +127,7 @@ def test_enabled_cors_static_route(self, cors_client): ), ) - assert result.headers['Access-Control-Allow-Methods'] == 'GET' + assert result.headers['Access-Control-Allow-Methods'] == 'GET, HEAD' assert result.headers['Access-Control-Allow-Headers'] == '*' assert result.headers['Access-Control-Max-Age'] == '86400' assert result.headers['Access-Control-Allow-Origin'] == '*' diff --git a/tests/test_static.py b/tests/test_static.py index 19dc4148d..5a39c74ee 100644 --- a/tests/test_static.py +++ b/tests/test_static.py @@ -1,5 +1,6 @@ import errno import io +import mimetypes import os import pathlib import posixpath @@ -53,29 +54,46 @@ def create_sr(asgi, prefix, directory, **kwargs): @pytest.fixture def patch_open(monkeypatch): def patch(content=None, validate=None, mtime=1736617934): - def open(path, mode): - class FakeFD(int): - pass - - class FakeStat: - def __init__(self, size): - self.st_size = size - self.st_mtime = mtime + class FakeStat: + def __init__(self, size): + self.st_size = size + self.st_mtime = mtime + def compute_stat_and_data(path): if validate: validate(path) - data = path.encode() if content is None else content + return FakeStat(len(data)), data + + def open(path, mode): + class FakeFD(int): + pass + + stat, data = compute_stat_and_data(path) fake_file = io.BytesIO(data) fd = FakeFD(1337) - fd._stat = FakeStat(len(data)) + fd._stat = stat fake_file.fileno = lambda: fd patch.current_file = fake_file return fake_file + def stat(path): + # NOTE: Used by HEAD requests, which stat the file directly + # instead of going through io.open()/os.fstat(). + stat_result, _data = compute_stat_and_data(path) + return stat_result + + # NOTE: Response/ResponseOptions lazily call mimetypes.init(), which + # itself calls the real os.stat() on a handful of well-known system + # paths. Trigger that now so it doesn't run (against our fake + # os.stat below) the first time a Response is constructed inside a + # test. + mimetypes.init() + monkeypatch.setattr(io, 'open', open) monkeypatch.setattr(os, 'fstat', lambda fileno: fileno._stat) + monkeypatch.setattr(os, 'stat', stat) patch.current_file = None return patch @@ -634,7 +652,167 @@ def test_options_request(client, patch_open): assert resp.status_code == 200 assert resp.text == '' assert int(resp.headers['Content-Length']) == 0 - assert resp.headers['Access-Control-Allow-Methods'] == 'GET' + assert resp.headers['Access-Control-Allow-Methods'] == 'GET, HEAD' + + +def test_head_request(client, patch_open): + patch_open(b'test_data') + + client.app.add_static_route('/static', '/var/www/statics') + + resp = client.simulate_request(method='HEAD', path='/static/foo/bar.txt') + assert resp.status == falcon.HTTP_200 + assert resp.text == '' + assert int(resp.headers['Content-Length']) == len(b'test_data') + assert resp.headers.get('Accept-Ranges') == 'bytes' + assert 'ETag' in resp.headers + assert 'Last-Modified' in resp.headers + + # NOTE: A HEAD request must not open (or otherwise leave dangling) a + # file handle, since the response never includes a body. + assert patch_open.current_file is None + + +def test_head_request_not_found(client, patch_open): + def validate(path): + raise OSError(errno.ENOENT, 'File not found') + + patch_open(validate=validate) + + client.app.add_static_route('/static', '/var/www/statics') + + resp = client.simulate_request(method='HEAD', path='/static/missing.txt') + assert resp.status == falcon.HTTP_404 + assert patch_open.current_file is None + + +def test_head_request_not_modified(client, patch_open): + patch_open(b'test_data') + + client.app.add_static_route('/static', '/var/www/statics') + + head_resp = client.simulate_request(method='HEAD', path='/static/foo/bar.txt') + etag = head_resp.headers['ETag'] + + resp = client.simulate_request( + method='HEAD', + path='/static/foo/bar.txt', + headers={'If-None-Match': etag}, + ) + assert resp.status == falcon.HTTP_304 + assert resp.text == '' + assert patch_open.current_file is None + + +@pytest.mark.parametrize( + 'range_header, exp_status, exp_content_range, exp_length', + [ + (None, falcon.HTTP_200, None, 16), + ('bytes=1-3', falcon.HTTP_206, 'bytes 1-3/16', 3), + ('bytes=-3', falcon.HTTP_206, 'bytes 13-15/16', 3), + ('bytes=8-', falcon.HTTP_206, 'bytes 8-15/16', 8), + ], +) +def test_head_request_range( + client, range_header, exp_status, exp_content_range, exp_length, patch_open +): + patch_open(b'0123456789abcdef') + + client.app.add_static_route('/downloads', '/opt/somesite/downloads') + + headers = {'Range': range_header} if range_header else None + resp = client.simulate_request( + method='HEAD', path='/downloads/thing.zip', headers=headers + ) + assert resp.status == exp_status + assert resp.text == '' + assert int(resp.headers['Content-Length']) == exp_length + assert resp.headers.get('Content-Range') == exp_content_range + assert patch_open.current_file is None + + +def test_head_request_range_not_satisfiable(client, patch_open): + patch_open(b'0123456789abcdef') + + client.app.add_static_route('/downloads', '/opt/somesite/downloads') + + resp = client.simulate_request( + method='HEAD', + path='/downloads/thing.zip', + headers={'Range': 'bytes=100-200'}, + ) + assert resp.status == falcon.HTTP_416 + assert resp.headers.get('Content-Range') == 'bytes */16' + assert patch_open.current_file is None + + +@pytest.mark.parametrize( + 'range_header', + ['bytes=1-3', 'bytes=-3', 'bytes=8-', 'bytes=0-30'], +) +def test_head_request_range_zero_length(client, range_header, patch_open): + patch_open(b'') + + client.app.add_static_route('/downloads', '/opt/somesite/downloads') + + resp = client.simulate_request( + method='HEAD', + path='/downloads/thing.zip', + headers={'Range': range_header}, + ) + assert resp.status == falcon.HTTP_200 + assert resp.text == '' + assert int(resp.headers['Content-Length']) == 0 + assert 'Content-Range' not in resp.headers + assert patch_open.current_file is None + + +def test_head_request_fallback_filename(client, patch_open, monkeypatch): + def validate(path): + if 'index' not in path: + raise OSError(errno.ENOENT, 'File not found') + + patch_open(validate=validate) + monkeypatch.setattr('os.path.isfile', lambda file: 'index' in file) + + client.app.add_static_route( + '/static', '/opt/somesite/static', fallback_filename='index.html' + ) + + resp = client.simulate_request(method='HEAD', path='/static/missing.txt') + assert resp.status == falcon.HTTP_200 + assert resp.text == '' + assert 'ETag' in resp.headers + assert patch_open.current_file is None + + +def test_head_request_fallback_filename_not_found(client, patch_open, monkeypatch): + def validate(path): + # NOTE: Neither the requested file nor the fallback file exist. + raise OSError(errno.ENOENT, 'File not found') + + patch_open(validate=validate) + monkeypatch.setattr('os.path.isfile', lambda file: 'index' in file) + + client.app.add_static_route( + '/static', '/opt/somesite/static', fallback_filename='index.html' + ) + + resp = client.simulate_request(method='HEAD', path='/static/missing.txt') + assert resp.status == falcon.HTTP_404 + assert patch_open.current_file is None + + +@pytest.mark.parametrize('method', ['POST', 'PUT', 'PATCH', 'DELETE']) +def test_method_not_allowed(client, method, patch_open): + patch_open(b'test_data') + + client.app.add_static_route('/static', '/var/www/statics') + + resp = client.simulate_request(method=method, path='/static/foo/bar.txt') + assert resp.status == falcon.HTTP_405 + assert resp.headers.get('Allow') == 'GET, HEAD' + assert patch_open.current_file is None def test_last_modified(client, patch_open):