Skip to content

Add disallowed_chars option to StaticRoute for custom filename filtering#2675

Open
zain-asif-dev wants to merge 3 commits into
falconry:masterfrom
zain-asif-dev:feat/static-route-configurable-disallowed-chars
Open

Add disallowed_chars option to StaticRoute for custom filename filtering#2675
zain-asif-dev wants to merge 3 commits into
falconry:masterfrom
zain-asif-dev:feat/static-route-configurable-disallowed-chars

Conversation

@zain-asif-dev

Copy link
Copy Markdown

Description

Fixes #1649.

StaticRoute hard-codes the set of characters it refuses to serve via a private _DISALLOWED_CHARS_PATTERN, which includes the tilde (~). This breaks legitimate use cases such as serving build output that legitimately contains a tilde in the filename (e.g. default~module.js from an Angular build), and the only workaround is monkeypatching a private attribute.

As discussed in the issue, this keeps the default behavior unchanged (tilde and friends are still disallowed unless a caller opts out), and adds a way to configure the set of disallowed characters:

  • Added a disallowed_chars parameter to StaticRoute.__init__ (and, correspondingly, to App.add_static_route()), which overrides the extra characters to disallow. It's interpreted as the body of a regex character class, matching how the existing pattern is built internally.
  • None (the default) preserves the previous behavior.
  • Passing an empty string disables this particular check altogether.
  • Made the previous hard-coded string public as StaticRoute.DEFAULT_DISALLOWED_CHARS, so callers can build on top of it (e.g. DEFAULT_DISALLOWED_CHARS.replace('~', '')) instead of having to hand-write the full character list.
  • Regardless of the above, the NUL byte and the Unicode replacement character (\ufffd) are always rejected via a separate, non-configurable check, since letting those through could result in surprising or unsafe behavior.

Testing

Added tests to tests/test_static.py:

  • test_disallowed_chars_default: passing None or the literal default string behaves the same as before (tilde still blocked).
  • test_disallowed_chars_override: overriding disallowed_chars to a narrower set of control-character ranges allows serving a file with a tilde in its name.
  • test_disallowed_chars_override_still_blocks_nul_and_replacement_char: even with an empty disallowed_chars override, the NUL byte and \ufffd are still rejected.
  • test_add_static_route_disallowed_chars: exercises the new parameter through App.add_static_route() end to end via the test client.

Ran the full existing tests/test_static.py suite (291 tests, including the 12 new ones) as well as the full test suite (4037 passed, 426 skipped, 1 pre-existing failure in test_asgi_servers.py::TestWebSocket unrelated to this change and reproducible on a clean checkout of master, likely due to a websockets library version mismatch in my local environment). Also ran ruff check, ruff format --check, and mypy against the changed files, all clean.

@CaselIT CaselIT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, change looks mostly fine.

Left a few suggestion and a question regarding impl, mainly for the other maintainers

Comment thread falcon/routing/static.py Outdated
Comment thread falcon/routing/static.py Outdated
Comment thread tests/test_static.py Outdated
Comment thread tests/test_static.py Outdated
Comment thread falcon/routing/static.py
@zain-asif-dev

Copy link
Copy Markdown
Author

Good catch, those notes were written by me for this PR, not vytas. Re-attributed them to myself.

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (4748b80) to head (f77a7bc).

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #2675   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           64        64           
  Lines         7985      7992    +7     
  Branches      1103      1105    +2     
=========================================
+ Hits          7985      7992    +7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

StaticRoute hard-codes a set of characters it refuses to serve,
including the tilde. This breaks legitimate use cases such as files
produced by build tools that include a tilde in the name (e.g.
default~module.js from an Angular build), forcing users to either
give up on falcon's static file serving or monkeypatch the private
_DISALLOWED_CHARS_PATTERN attribute.

Fixes falconry#1649.

Added a disallowed_chars constructor parameter (and the matching
add_static_route() argument) that overrides the set of extra
characters to disallow, interpreted as the body of a regex character
class. Passing None (the default) keeps the previous behavior via the
newly public DEFAULT_DISALLOWED_CHARS attribute, and passing an empty
string disables the extra check entirely. The NUL byte and the Unicode
replacement character are always rejected regardless of this setting,
since letting those through could result in surprising behavior.

This matches the approach discussed in the issue: keep serving tilde
files disallowed by default so this isn't a breaking change, but let
callers opt into a different set of disallowed characters.

Signed-off-by: Zain Asif <zainasif.jutt1@gmail.com>
Signed-off-by: Zain Asif <zainasif.jutt1@gmail.com>
Signed-off-by: Zain Asif <zainasif.jutt1@gmail.com>
@zain-asif-dev zain-asif-dev force-pushed the feat/static-route-configurable-disallowed-chars branch from 32d8add to f77a7bc Compare July 13, 2026 14:00
@zain-asif-dev

Copy link
Copy Markdown
Author

Combined the two regex checks into one: the always-disallowed NUL byte and replacement character are now folded into _disallowed_chars_pattern at construction time, so match() only performs a single regex search. Also rebased onto latest master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Serve static discard files with a tilde(~) in them even if valid

2 participants