Commit Graph
2 Commits
Author SHA1 Message Date
Rohit Behera 1880021e76 network_proxy: reject proxy URLs that resolve to a port with no host (#7922)
* network_proxy: reject proxy URLs that resolve to a port with no host

The host check after placeholder replacement had its arguments swapped:

    strings.Split("", pUrl.Host)[0] == ":"

This splits the empty string using pUrl.Host as the separator, so it
returns [""] and element 0 is always "", never ":". The comparison was
dead for every possible host value, which meant the "http://:80" case
named in the comment directly below it was never rejected. Such a URL
was handed back as the proxy, and the failure surfaced later as a
confusing dial error instead of the intended message.

Only the pUrl.Host == "" half of the condition ever did anything, so
"/some/path" was still caught.

Use url.URL.Hostname(), which returns the host with any port stripped
and is "" for exactly the two cases the comment describes -- ":80" and
"" -- while leaving IPv6 literals such as "[::1]:80" and userinfo forms
intact. That collapses both clauses into one expression.

Also adds tests for this function; the package previously had none.

* network_proxy: cover userinfo-only and IPv6 hosts in the tests

Differential-tested the old predicate against the new one across 36 URL
forms. Every behavioural change is in the same direction -- previously
accepted, now rejected -- and all of them are host-less. Nothing that was
rejected before is accepted now, and no legitimate host form changes.

Two of those forms were worth pinning down in the test:

  - "http://user:pass@:8080" parses with Host ":8080", so it is just as
    host-less as "http://:80". The comment in the source doesn't name
    this variant, but it was accepted before and is rejected now.

  - IPv6 literals are full of colons, so a repair that split Host on ":"
    rather than using Hostname() could plausibly reject them. Added
    "[::1]:8080" and "[2001:db8::1]" as regression guards; both are
    accepted before and after, which is the point.

On unfixed master the port-only and userinfo cases both fail; the IPv6
cases pass on both sides.
2026-08-10 15:58:43 +10:00
173573035c core: add modular network_proxy support (#6399)
* core: add modular `network_proxy` support

Co-authored-by: @ImpostorKeanu
Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>

* move modules around

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>

* add caddyfile implementation

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>

* address feedbcak

* Apply suggestions from code review

Co-authored-by: Francis Lavoie <lavofr@gmail.com>

* adapt ForwardProxyURL to use the NetworkProxyRaw

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>

* remove redundant `url`  in log

Co-authored-by: Matt Holt <mholt@users.noreply.github.com>

* code review

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>

* remove `.source` from the module ID

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>

---------

Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
Co-authored-by: Matt Holt <mholt@users.noreply.github.com>
2025-03-21 17:06:15 +00:00