Match domain sub-applications on every spelling of the registered host - #13693
Match domain sub-applications on every spelling of the registered host#13693rodrigobnogueira wants to merge 2 commits into
Conversation
psf-chronographer
Bot
added
the
bot:chronographer:provided
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #13693 +/- ##
=======================================
Coverage 99.03% 99.03%
=======================================
Files 135 135
Lines 50940 50960 +20
Branches 2677 2678 +1
=======================================
+ Hits 50446 50466 +20
Misses 370 370
Partials 124 124
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Merging this PR will improve performance by 14.07%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_link_param_pattern_redos_payload[embedded_newlines] |
40 µs | 35.1 µs | +14.07% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing rodrigobnogueira:domain-host-normalization (4712a95) with master (5da6d53)
Footnotes
-
83 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
f570b51 to
c22594e
Compare
add_domain() stores the domain lowercased, without a trailing dot and without the default port, but matched a request against the raw Host header with nothing more than lower(). A Host of example.com. or example.com:80 therefore never matched example.com and the request fell through to the parent application's routes, which is not what a domain application is for. The request side now applies the same normalization before comparing, for both Domain and MaskDomain. Only ASCII digits count as a port, since that is all the registration side accepts, so a Host with a port spelled in other digits stays unmatched rather than widening the domain.
c22594e to
4712a95
Compare
Confidence Score: 4/5Not safe to merge until oversized numeric Host ports no longer interrupt domain route resolution. A reproduced routing failure remains in the changed host-normalization path. Files Needing Attention: aiohttp/web_urldispatcher.py
 T-Rex LogsWhat T-Rex did
Comments Outside Diff (1)
Reviews (1): Last reviewed commit: "Add change note" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| name, sep, port = host.rpartition(":") | ||
| if sep and port.isascii() and port.isdigit(): | ||
| name = name.rstrip(".") | ||
| port_number = int(port) |
There was a problem hiding this comment.
A client can send a valid-sized Host header with a numeric port longer than Python's integer-string limit. This unbounded int(port) conversion raises ValueError while exact and wildcard domain sub-applications resolve the request. The exception prevents the normal unmatched-host fallback, causing routing to fail rather than return a 404 response.
Knowledge Base Used: Server routing, responses, and static files
Artifacts
- This executable script builds exact and wildcard domain subapplications and resolves a request whose numeric Host port is one digit beyond Python's conversion limit, exercising the affected path.
- This captured execution runs the reproducer against pre-PR commit 5da6d53 and shows both domain forms return HTTP 404 Not Found, establishing the no-match baseline.
- This captured execution runs the reproducer against the PR implementation with the 4,301-digit numeric port and records the focused runtime invocation.
1 participant
What do these changes do?
add_domain()stores the domain lowercased, without a trailing dot and without the default port, but matched a request against the rawHostheader with nothing more thanlower(). AHostofexample.com.orexample.com:80therefore never matched a domain registered asexample.com, and the request fell through to the parent application's routes.The request side now applies the same normalization before comparing, for both
DomainandMaskDomain. Only ASCII digits count as a port:"٨٠".isdigit()is true andint("٨٠")is 80, so a plainisdigit()check would have let aHostwith a port in other digits match the domain, which the registration side never accepts.Are there changes in behavior for the user?
A domain application now receives requests whose
Hostspells its domain with a trailing dot, an explicit:80, or uppercase letters. Previously those reached the parent application instead. A different port, including:443, or a port that is not a number, still does not match.MaskDomainmatching becomes case-insensitive as a result, matchingDomain: it previously ran its regex against the rawHost, soA.EXAMPLE.COMdid not match*.example.comwhile it did match a plainDomain.Is it a substantial burden for the maintainers to support this?
No. One helper on
Domain, shared withMaskDomain.Related issue number
None.
Checklist
CONTRIBUTORS.txtCHANGES/folder