Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/_newsfragments/1649.newandimproved.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
:class:`~falcon.routing.StaticRoute` (and, correspondingly,
:meth:`~falcon.App.add_static_route`) now accept a ``disallowed_chars``
argument, letting an app override the default set of characters
disallowed in requested filenames (previously hard-coded as
``_DISALLOWED_CHARS_PATTERN``, now exposed as
:attr:`~falcon.routing.StaticRoute.DEFAULT_DISALLOWED_CHARS`). This makes
it possible, for instance, to serve files whose names legitimately
contain a tilde (``~``). The NUL byte and the Unicode replacement
character are always disallowed, regardless of this setting.
7 changes: 7 additions & 0 deletions falcon/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,7 @@ def add_static_route(
directory: str | pathlib.Path,
downloadable: bool = False,
fallback_filename: str | None = None,
disallowed_chars: str | None = None,
) -> None:
"""Add a route to a directory of static files.

Expand Down Expand Up @@ -786,6 +787,11 @@ def add_static_route(
fallback_filename (str): Fallback filename used when the requested file
is not found. Can be a relative path inside the prefix folder or
any valid absolute path.
disallowed_chars (str): A string overriding the set of
additional characters to disallow in the requested
filename, in place of
:attr:`~falcon.routing.StaticRoute.DEFAULT_DISALLOWED_CHARS`.
See :class:`~falcon.routing.StaticRoute` for more details.

"""

Expand All @@ -794,6 +800,7 @@ def add_static_route(
directory,
downloadable=downloadable,
fallback_filename=fallback_filename,
disallowed_chars=disallowed_chars,
)
self._static_routes.insert(0, (sr, sr, False))
self._update_sink_and_static_routes()
Expand Down
46 changes: 44 additions & 2 deletions falcon/routing/static.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,13 +168,40 @@ class StaticRoute:
Content-Disposition header (provided it was requested with the
`downloadable` parameter described above), are derived from the
fallback filename, as opposed to the requested filename.
disallowed_chars (str): A string overriding the set of additional
characters to disallow in the requested filename, in place of
:attr:`DEFAULT_DISALLOWED_CHARS`. The string is interpreted as
the body of a regex character class (i.e., it is inserted
verbatim between the square brackets when compiling the
pattern used to search the requested filename), so characters
that are special to regular expressions (such as ``]``,
``\\`` or ``^``) must be escaped accordingly. Pass an empty
string to disable this check altogether.

Defaults to ``None``, in which case
:attr:`DEFAULT_DISALLOWED_CHARS` is used unmodified. Regardless
of this setting, the NUL byte (``'\\x00'``) and the Unicode
replacement character (``'\\ufffd'``) are always disallowed.
"""

#: A string of characters disallowed in requested filenames by default
#: (unless customized via the `disallowed_chars` constructor
#: parameter). The string is interpreted as the body of a regex
#: character class.
DEFAULT_DISALLOWED_CHARS: ClassVar[str] = '\x00-\x1f\x80-\x9f\ufffd~?<>:*|\'"'

# NOTE(kgriffs): Don't allow control characters and reserved chars
_DISALLOWED_CHARS_PATTERN: ClassVar[Pattern[str]] = re.compile(
'[\x00-\x1f\x80-\x9f\ufffd~?<>:*|\'"]'
'[' + DEFAULT_DISALLOWED_CHARS + ']'
)

# NOTE(zain-asif-dev): Regardless of any disallowed_chars override,
# always reject the NUL byte and the Unicode replacement character,
# since allowing them could result in unexpected or surprising
# behavior. Folded into _disallowed_chars_pattern in __init__ below,
# so that match() only ever needs to perform a single regex search.
_ALWAYS_DISALLOWED_CHARS: ClassVar[str] = '\x00\ufffd'

# NOTE(vytas): Match the behavior of the underlying os.path.normpath.
_DISALLOWED_NORMALIZED_PREFIXES: ClassVar[tuple[str, ...]] = (
'..' + os.path.sep,
Expand All @@ -191,6 +218,7 @@ def __init__(
directory: str | Path,
downloadable: bool = False,
fallback_filename: str | None = None,
disallowed_chars: str | None = None,
) -> None:
if not prefix.startswith('/'):
raise ValueError("prefix must start with '/'")
Expand All @@ -208,6 +236,20 @@ def __init__(
if not os.path.isfile(self._fallback_filename):
raise ValueError('fallback_filename is not a file')

if disallowed_chars is None:
self._disallowed_chars_pattern = self._DISALLOWED_CHARS_PATTERN
elif disallowed_chars:
self._disallowed_chars_pattern = re.compile(
'[' + disallowed_chars + self._ALWAYS_DISALLOWED_CHARS + ']'
)
else:
# NOTE(zain-asif-dev): An explicitly empty string means the
# caller does not want any additional characters disallowed,
# beyond the ones in _ALWAYS_DISALLOWED_CHARS above.
self._disallowed_chars_pattern = re.compile(
'[' + self._ALWAYS_DISALLOWED_CHARS + ']'
)

# NOTE(kgriffs): Ensure it ends with a path separator to ensure
# we only match on the complete segment. Don't raise an error
# because most people won't expect to have to append a slash.
Expand Down Expand Up @@ -240,7 +282,7 @@ def __call__(self, req: Request, resp: Response, **kw: Any) -> None:
if (
not (without_prefix or self._fallback_filename is not None)
or without_prefix.strip().rstrip('.') != without_prefix
or self._DISALLOWED_CHARS_PATTERN.search(without_prefix)
or self._disallowed_chars_pattern.search(without_prefix)
Comment thread
zain-asif-dev marked this conversation as resolved.
or '\\' in without_prefix
or '//' in without_prefix
or len(without_prefix) > self._MAX_NON_PREFIXED_LEN
Expand Down
108 changes: 108 additions & 0 deletions tests/test_static.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,114 @@ async def run():
assert resp.headers.get('accept-ranges') == 'bytes'


@pytest.mark.parametrize(
'disallowed_chars',
[
None,
# NOTE(zain-asif-dev): Equivalent to the default, just spelled out
# explicitly to make sure passing it verbatim still works.
'\x00-\x1f\x80-\x9f\ufffd~?<>:*|\'"',
],
)
def test_disallowed_chars_default(asgi, util, disallowed_chars, patch_open):
patch_open(b'')

sr = create_sr(
asgi, '/static', '/var/www/statics', disallowed_chars=disallowed_chars
)

req = util.create_req(
asgi,
host='test.com',
path='/static/~/.ssh/authorized_keys',
root_path='statics',
)
resp = util.create_resp(asgi)

with pytest.raises(falcon.HTTPNotFound):
if asgi:
falcon.async_to_sync(sr, req, resp)
else:
sr(req, resp)


def test_disallowed_chars_override(asgi, util, patch_open):
patch_open()

# NOTE(zain-asif-dev): Override the default set of disallowed characters
# to allow tildes in requested filenames (see also GH #1649), while
# still disallowing control characters.
sr = create_sr(
asgi,
'/static',
'/var/www/statics',
disallowed_chars='\x00-\x1f\x80-\x9f',
)

req = util.create_req(
asgi,
host='test.com',
path='/static/default~module.js',
root_path='statics',
)
resp = util.create_resp(asgi)

if asgi:

async def run():
await sr(req, resp)
return await resp.stream.read()

body = falcon.async_to_sync(run)
else:
sr(req, resp)
body = resp.stream.read()

assert body.decode() == normalize_path('/var/www/statics/default~module.js')


@pytest.mark.parametrize(
'uri',
[
'/static/.\x00ssh/authorized_keys',
'/static/\ufffdsomething',
],
)
def test_disallowed_chars_override_still_blocks_nul_and_replacement_char(
asgi, util, uri, patch_open
):
patch_open(b'')

# NOTE(zain-asif-dev): Even when disallowed_chars is overridden with an
# "empty" pattern, the NUL byte and the Unicode replacement
# character must still be rejected.
sr = create_sr(asgi, '/static', '/var/www/statics', disallowed_chars='')

req = util.create_req(asgi, host='test.com', path=uri, root_path='statics')
resp = util.create_resp(asgi)

with pytest.raises(falcon.HTTPNotFound):
if asgi:
falcon.async_to_sync(sr, req, resp)
else:
sr(req, resp)


def test_add_static_route_disallowed_chars(client, patch_open):
patch_open()

client.app.add_static_route(
'/static',
normalize_path('/var/www/statics'),
disallowed_chars='\x00-\x1f\x80-\x9f',
)

result = client.simulate_get('/static/default~module.js')

assert result.status_code == 200
assert result.text == normalize_path('/var/www/statics/default~module.js')


@pytest.mark.parametrize(
'range_header, exp_content_range, exp_content',
[
Expand Down