fix(tornado): match http routes in correct order and missing routes certain regex patterns#17515
Open
florentinl wants to merge 5 commits intomainfrom
Open
fix(tornado): match http routes in correct order and missing routes certain regex patterns#17515florentinl wants to merge 5 commits intomainfrom
florentinl wants to merge 5 commits intomainfrom
Conversation
Codeowners resolved as |
…mplex patterns In nested Tornado applications, _find_route used deque.extendleft without reversing the sub-rules first, causing them to be processed in reverse declaration order. A catch-all route declared after a specific one would incorrectly win. Also introduces _regex_to_route, a cached helper that populates http.route even for patterns Tornado cannot reverse (e.g. those containing non-capturing groups). Capturing groups are normalised to %s to stay consistent with Tornado's own PathMatches._path format.
florentinl
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CodeQL flagged RouteMixedPatternHandler as a reflected XSS sink because it wrote the URL-captured item_id directly to the response. The handler only exists to verify route matching; use a static response instead.
florentinl
requested review from
KowalskiThomas,
Kyle-Verhoog,
brettlangdon and
dubloom
KowalskiThomas
approved these changes
Apr 14, 2026
Contributor
KowalskiThomas
left a comment
KowalskiThomas
left a comment
There was a problem hiding this comment.
Release notes make sense to me.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f962375805
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ute parsing
_regex_to_route now skips over [...] character classes verbatim so that
'(' inside a class is not mistaken for a capturing group boundary.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
brettlangdon
approved these changes
Apr 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Two fixes to
_find_route, the helper that determines thehttp.routetag for Tornado requests.1. Route matching order in nested applications
_find_routepushes sub-rules of a nestedApplicationonto the front of the processing deque withextendleft. Becauseextendleftreverses the input sequence as it inserts, the routes were processed in reverse declaration order — a catch-all defined after a specific route would incorrectly win. Fix:extendleft(reversed(rule.target.rules)).2.
http.routemissing for complex regex patternsTornado's
PathMatches._pathisNonewhenever the route regex cannot be reversed (e.g. patterns containing non-capturing groups(?:...), lookaheads, or lookbehinds). Previouslyhttp.routewas absent for those routes. This adds_regex_to_route, alru_cache-backed helper that converts any regex pattern to a route string: capturing groups become%s(consistent with Tornado's own format), non-capturing constructs are kept verbatim.Both issues were surfaced by Streamlit, which mounts a nested Tornado application with non-trivial route patterns.
Testing
test_nested_application_route_order: nested app with a specific route before a catch-all; asserts the specific route wins. Fails without fix 1.test_complex_pattern_route: covers a pure non-capturing group route and a mixed non-capturing + capturing group route; assertshttp.routeis populated and capturing groups are rendered as%s. Fails without fix 2.test_regex_to_route: parametrised unit tests for_regex_to_routein isolation, covering anchors, positional/named capturing groups, non-capturing groups, lookaheads, nested groups, and escaped parens.Risks
None. Both changes only affect route tag computation; request dispatch is handled by Tornado independently.
http.routevalues for cases that were previously not supported.Additional Notes
N/A