Repository navigation
Potential issue: default-port normalization depends on scheme case ("HTTP://example.org:80/" keeps its port) #3817
Replies: 1 comment
|
Confirmed. I reckon the cause is one line: the scheme is lowercased for storage, but the original scheme string is what gets passed to port normalization. To dive deeper: In urlparse() the parsed scheme is computed as scheme.lower(), yet the next line calls normalize_port(port, scheme) with the raw value. normalize_port() looks the default port up in a lowercase-keyed dict (for example "ftp" maps to 21, "http" maps to 80, and so on), so an uppercase "HTTP" misses and the default-port rule never applies: httpx.URL with "http://example.org:80/" has port None. The inconsistency is visible inside a single object. URL.scheme reports the normalized "http", while URL.port still carries 80 and str(url) prints it, so the two forms of the same URL do not compare equal even though the scheme normalizes identically. Only the uppercase input is affected. A scheme that is already lowercase, or mixed case that happens to be lowercase after .lower(), takes the normal path. Propose fix: pass the already-lowercased parsed_scheme instead of the raw scheme to normalize_port(), or lowercase inside normalize_port() before the dict lookup, so default-port elision is decided on the same normalized scheme that is stored in the result. |
Uh oh!
There was an error while loading. Please reload this page.
Raising this here per the contributing guide, as PR creation on this repo is limited to collaborators. Found with property-based testing (round-trip and normalization invariants over generated URLs).
Default-port normalization currently depends on the case of the scheme, because urlparse() passes the raw scheme to normalize_port() instead of the lowercased one:
The string form of the first URL is not even a fixpoint: str(u) re-parses to a different URL, and copy_with() silently drops the port. The URL class docstring presents an uppercase scheme ("HTTPS://...") as parsing into normalized components, and states that the scheme "is normalized to always be lowercased" — the port lookup is the one place that still uses the un-normalized form.
Schemes are case-insensitive and are normalized to lowercase everywhere else in the parser, so the port lookup should use the normalized scheme too.
I have the one-line fix plus a regression test ready on a fork branch: https://github.com/Sun-Wukhan/httpx/tree/fix/url-scheme-case-port (the full tests/models suite passes with it). Happy for a maintainer to open the PR from it, or to open one myself if the restriction is lifted for it.
All reactions