URL.build: reject bool port with a clear TypeError before netloc is built#1786
URL.build: reject bool port with a clear TypeError before netloc is built#1786HrachShah wants to merge 2 commits into
Conversation
Merging this PR will degrade performance by 9.88%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | test_url_build_encoded_with_host_and_port |
254.4 µs | 282.3 µs | -9.88% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing HrachShah:patch-3 (de95bb5) with master (94274c2)1
Footnotes
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1786 +/- ##
=======================================
Coverage 99.86% 99.86%
=======================================
Files 21 21
Lines 4310 4315 +5
Branches 252 252
=======================================
+ Hits 4304 4309 +5
Misses 4 4
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Coverage Report for CI Build 28830767677Coverage increased (+0.001%) to 98.741%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Summary
URL.build(..., port=True)(orport=False) used to slip past theisinstance(port, int)check becauseboolis a subclass ofint.The port got stringified to
"True"/"False"bymake_netlocand the resulting URL looked valid at construction time but blew up
deep inside
split_netlocthe first time the URL was rendered orits
explicit_port/raw_hostwas accessed:The user had no way to know from the message that the port was the
problem — it pointed at
split_netloc7 frames deep, not at theport=Truethey passed toURL.build.Fix
Mirror the bool-rejection in
with_port(which already hasisinstance(port, bool) or not isinstance(port, int)) on theURL.buildconstructor:Now
URL.build(port=True)raisesTypeError: The port is required to be int, got <class 'bool'>.at the call site, before any netlocis built and before the URL is constructed.
Test
New test in
tests/test_url.py:Verified the test fails on baseline with
ValueError: Can't build URL with "port" but without "host".(the
if port and not hostcheck fires next becauseTrueistruthy and
host=""by default), and passes on the fix.All 510 existing yarl tests still pass.
Precedent
The same bool-subclass-of-int footgun is already rejected in
URL.with_port(line 1189 of_url.py). This PR bringsURL.buildin line with the same defensive check.