diff --git a/README.md b/README.md index 4562b7b..6d7c017 100644 --- a/README.md +++ b/README.md @@ -303,7 +303,7 @@ Saving settings applies most of them immediately. How each group takes effect: | `controllerMethod` (all five channels) | Every channel is **rebuilt**: the running controllers are stopped and a fresh set is built from the new settings. A channel that owns a connection reconnects — Telegram re-runs its `getMe` handshake, NapCat opens a new WebSocket — so notifications sent during the swap are lost. The rebuild only happens when this section actually changed; saving a message template does not disturb the channels. | | `controllerMessage`, `debug`, `bot_user_config.json`, `bot_node_config.json`, `webhook.endpoints` | Picked up as they are used; nothing is restarted. | | `system.debugMode` | Applies to both behavior and log filtering. | -| `system.networkProxy` | Read when a channel builds its connection, so a channel only sees a new value once it is rebuilt. Changing this key **alone** does not trigger the rebuild above — change a `controllerMethod` value as well, or restart. | +| `system.networkProxy` | Read on every request and every dial, so a new address reaches channels that are already running. Only the per-channel `networkUseProxy` opt-in is fixed when a channel is built, so toggling that still needs the rebuild above. | | `system.listenAddr` / `listenPort`, `webhook.enabled` / `listenAddr` / `listenPort`, `dataPath`, `dbPath`, `komari.dashboardURL` | **Applied at startup only.** Saving them changes the file and the in-memory configuration but not the running listener, database or Komari client — restart to apply. | ## API diff --git a/internal/netproxy/netproxy.go b/internal/netproxy/netproxy.go index 8fe0bdf..0d77712 100644 --- a/internal/netproxy/netproxy.go +++ b/internal/netproxy/netproxy.go @@ -2,6 +2,10 @@ // network proxy configured in the system config. Each caller decides whether // to use the proxy by passing its own useProxy flag (the per-channel // networkUseProxy setting), so proxying is opt-in per channel. +// +// The opt-in is captured when a client is built, but the proxy address is not: +// it is read again on every request and every dial, so editing +// system.networkProxy takes effect on clients that already exist. package netproxy import ( @@ -39,19 +43,23 @@ func proxyURL() *url.URL { } // ProxyFunc returns a transport proxy function that routes requests through -// the configured network proxy when enabled. It returns nil when the caller -// opts out or no proxy is configured, meaning direct connection. The returned -// function is compatible with both http.Transport.Proxy and -// websocket.Dialer.Proxy. +// the configured network proxy when enabled, and nil when the caller opts out +// of proxying entirely. The returned function is compatible with both +// http.Transport.Proxy and websocket.Dialer.Proxy. +// +// The proxy address is resolved on every call rather than once here, so a +// settings update that changes system.networkProxy reaches a client that was +// already built. That is also why opting out is the only case that returns nil: +// a function resolved to nothing at construction time would pin its client to +// whatever was configured then. A nil URL from the returned function means no +// proxy is configured and the request goes direct. func ProxyFunc(useProxy bool) func(*http.Request) (*url.URL, error) { if !useProxy { return nil } - u := proxyURL() - if u == nil { - return nil + return func(*http.Request) (*url.URL, error) { + return proxyURL(), nil } - return http.ProxyURL(u) } // HTTPClient builds an http.Client that sends traffic through the configured @@ -71,10 +79,16 @@ func HTTPClient(useProxy bool, timeout time.Duration) *http.Client { // through the configured HTTP CONNECT proxy when enabled. Its signature // matches net.DialTimeout so it can be plugged into gomail's NetDialTimeout // to send SMTP over the proxy. +// +// Like ProxyFunc it reads the proxy address per dial, so clearing or changing +// system.networkProxy reaches a dialer that already exists. func DialWithTimeout(useProxy bool) func(network, addr string, timeout time.Duration) (net.Conn, error) { - u := proxyURL() return func(network, addr string, timeout time.Duration) (net.Conn, error) { - if !useProxy || u == nil { + if !useProxy { + return net.DialTimeout(network, addr, timeout) + } + u := proxyURL() + if u == nil { return net.DialTimeout(network, addr, timeout) } return dialViaProxy(u, addr, timeout) diff --git a/internal/netproxy/netproxy_test.go b/internal/netproxy/netproxy_test.go new file mode 100644 index 0000000..3e48178 --- /dev/null +++ b/internal/netproxy/netproxy_test.go @@ -0,0 +1,133 @@ +package netproxy + +import ( + "net" + "net/http" + "net/url" + "os" + "path/filepath" + "testing" + "time" + + "nukumizu-backend/config" +) + +// publishConfig writes a config.json and makes it the configuration in effect, +// which is what a settings update does. +func publishConfig(t *testing.T, body string) { + t.Helper() + path := filepath.Join(t.TempDir(), "config.json") + if err := os.WriteFile(path, []byte(body), 0o644); err != nil { + t.Fatalf("write temp config: %v", err) + } + if _, err := config.LoadGlobalConfig(path); err != nil { + t.Fatalf("LoadGlobalConfig: %v", err) + } +} + +// resolve runs a proxy function and returns the URL it chose, or "" when it +// chose a direct connection. +func resolve(t *testing.T, proxy func(*http.Request) (*url.URL, error)) string { + t.Helper() + req, err := http.NewRequest(http.MethodGet, "https://example.com/", nil) + if err != nil { + t.Fatalf("NewRequest: %v", err) + } + u, err := proxy(req) + if err != nil { + t.Fatalf("proxy function: %v", err) + } + if u == nil { + return "" + } + return u.String() +} + +// TestProxyFuncResolvesPerCall is the property that makes system.networkProxy +// hot-reloadable: the function handed to a transport keeps reading the live +// configuration instead of the address that was configured when it was built. +func TestProxyFuncResolvesPerCall(t *testing.T) { + publishConfig(t, `{"system":{"networkProxy":"http://127.0.0.1:7890"}}`) + + proxy := ProxyFunc(true) + if proxy == nil { + t.Fatal("ProxyFunc(true) returned nil, so the channel would never proxy") + } + if got := resolve(t, proxy); got != "http://127.0.0.1:7890" { + t.Errorf("first resolution = %q", got) + } + + // The same function must follow a settings update. + publishConfig(t, `{"system":{"networkProxy":"http://127.0.0.1:8888"}}`) + if got := resolve(t, proxy); got != "http://127.0.0.1:8888" { + t.Errorf("after a settings update the same function resolved %q", got) + } + + // Clearing the proxy falls back to a direct connection. + publishConfig(t, `{"system":{"networkProxy":""}}`) + if got := resolve(t, proxy); got != "" { + t.Errorf("a cleared proxy still resolved %q", got) + } +} + +func TestProxyFuncOptOutReturnsNil(t *testing.T) { + publishConfig(t, `{"system":{"networkProxy":"http://127.0.0.1:7890"}}`) + + // A channel with networkUseProxy off must not be handed a function at all, + // so its transport keeps the default direct dialing. + if ProxyFunc(false) != nil { + t.Error("ProxyFunc(false) must return nil") + } +} + +func TestProxyFuncNormalizesMissingScheme(t *testing.T) { + publishConfig(t, `{"system":{"networkProxy":"127.0.0.1:7890"}}`) + + if got := resolve(t, ProxyFunc(true)); got != "http://127.0.0.1:7890" { + t.Errorf("resolved %q, want the http:// prefix added", got) + } +} + +func TestProxyFuncIgnoresUnusableProxy(t *testing.T) { + // A value that cannot be parsed must leave the client dialing directly + // rather than failing every request. + publishConfig(t, `{"system":{"networkProxy":"://missing-scheme"}}`) + + if got := resolve(t, ProxyFunc(true)); got != "" { + t.Errorf("an unparseable proxy resolved %q, want a direct connection", got) + } +} + +// TestDialWithTimeoutDialsDirectlyWithoutProxy covers the path a cleared +// system.networkProxy takes: the dialer was built while a proxy was configured, +// and must fall back to a direct dial once there is none. +func TestDialWithTimeoutDialsDirectlyWithoutProxy(t *testing.T) { + publishConfig(t, `{"system":{"networkProxy":"http://127.0.0.1:7890"}}`) + + dial := DialWithTimeout(true) + if dial == nil { + t.Fatal("DialWithTimeout(true) returned nil") + } + + // No proxy is listening on that address, so a dial attempted now would + // fail; clearing the setting is what makes the direct path reachable. + publishConfig(t, `{"system":{"networkProxy":""}}`) + + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + defer ln.Close() + go func() { + conn, err := ln.Accept() + if err == nil { + conn.Close() + } + }() + + conn, err := dial("tcp", ln.Addr().String(), 5*time.Second) + if err != nil { + t.Fatalf("dial through a cleared proxy: %v", err) + } + conn.Close() +}