mirror of
https://github.com/caddyserver/caddy.git
synced 2026-09-15 15:17:25 -04:00
Rohit Behera
1880021e76
* 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.
115 lines
3.0 KiB
Go
115 lines
3.0 KiB
Go
package network
|
|
|
|
import (
|
|
"context"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
|
|
"go.uber.org/zap"
|
|
|
|
"github.com/caddyserver/caddy/v2"
|
|
)
|
|
|
|
// TestProxyFromURLPlaceholderHostValidation checks that a network_proxy URL
|
|
// which resolves to a value without a host is rejected after placeholders are
|
|
// applied. url.Parse accepts both "http://:80" and "/some/path" without
|
|
// returning an error, so ProxyFunc has to reject them itself.
|
|
func TestProxyFromURLPlaceholderHostValidation(t *testing.T) {
|
|
for i, tc := range []struct {
|
|
name string
|
|
url string
|
|
hostRepl string
|
|
wantHost string
|
|
wantErr bool
|
|
}{
|
|
{
|
|
name: "port only, no host",
|
|
url: "http://{proxy.host}:80",
|
|
hostRepl: "",
|
|
wantErr: true,
|
|
},
|
|
{
|
|
name: "no scheme or host, path only",
|
|
url: "{proxy.host}/some/path",
|
|
hostRepl: "",
|
|
wantErr: true,
|
|
},
|
|
{
|
|
// url.Parse puts the userinfo elsewhere, so Host is still just
|
|
// ":8080" here. The comment above the check doesn't name this
|
|
// form, but it is equally host-less and equally unusable.
|
|
name: "userinfo but no host",
|
|
url: "http://user:pass@{proxy.host}:8080",
|
|
hostRepl: "",
|
|
wantErr: true,
|
|
},
|
|
{
|
|
name: "host and port",
|
|
url: "http://{proxy.host}:8080",
|
|
hostRepl: "proxy.example.com",
|
|
wantHost: "proxy.example.com",
|
|
wantErr: false,
|
|
},
|
|
{
|
|
name: "host without port",
|
|
url: "http://{proxy.host}",
|
|
hostRepl: "proxy.example.com",
|
|
wantHost: "proxy.example.com",
|
|
wantErr: false,
|
|
},
|
|
{
|
|
// Guards against a repair that splits Host on ":" instead of
|
|
// using Hostname(): an IPv6 literal is full of colons and must
|
|
// not be mistaken for a missing host.
|
|
name: "IPv6 literal with port",
|
|
url: "http://[{proxy.host}]:8080",
|
|
hostRepl: "::1",
|
|
wantHost: "::1",
|
|
wantErr: false,
|
|
},
|
|
{
|
|
name: "IPv6 literal without port",
|
|
url: "http://[{proxy.host}]",
|
|
hostRepl: "2001:db8::1",
|
|
wantHost: "2001:db8::1",
|
|
wantErr: false,
|
|
},
|
|
} {
|
|
p := ProxyFromURL{URL: tc.url, logger: zap.NewNop()}
|
|
|
|
repl := caddy.NewReplacer()
|
|
repl.Set("proxy.host", tc.hostRepl)
|
|
|
|
req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
|
|
req = req.WithContext(context.WithValue(req.Context(), caddy.ReplacerCtxKey, repl))
|
|
|
|
proxyURL, err := p.ProxyFunc()(req)
|
|
|
|
if tc.wantErr {
|
|
if err == nil {
|
|
t.Errorf("Test %d (%s): expected an error for %q but got none (proxy URL: %v)",
|
|
i, tc.name, tc.url, proxyURL)
|
|
}
|
|
if proxyURL != nil {
|
|
t.Errorf("Test %d (%s): expected a nil proxy URL for %q, got %v",
|
|
i, tc.name, tc.url, proxyURL)
|
|
}
|
|
continue
|
|
}
|
|
|
|
if err != nil {
|
|
t.Errorf("Test %d (%s): unexpected error for %q: %v", i, tc.name, tc.url, err)
|
|
continue
|
|
}
|
|
if proxyURL == nil {
|
|
t.Errorf("Test %d (%s): expected a proxy URL for %q, got nil", i, tc.name, tc.url)
|
|
continue
|
|
}
|
|
if proxyURL.Hostname() != tc.wantHost {
|
|
t.Errorf("Test %d (%s): expected host %q, got %q",
|
|
i, tc.name, tc.wantHost, proxyURL.Hostname())
|
|
}
|
|
}
|
|
}
|