diff --git a/cli/install.go b/cli/install.go index d8fbb26d2dd2243872611c7488102942283887c7..59844f49aedbddcc158e16c3bbeffbff1d5a2d3b 100644 --- a/cli/install.go +++ b/cli/install.go @@ -7,7 +7,8 @@ import ( "os" "path/filepath" "strings" - "time" + + "github.com/floatpane/matcha/internal/httpclient" ) // RunInstall handles `matcha install `. @@ -22,7 +23,7 @@ func RunInstall(args []string) error { if strings.HasPrefix(source, "http://") || strings.HasPrefix(source, "https://") { // Download from URL - client := &http.Client{Timeout: 30 * time.Second} + client := httpclient.New(httpclient.InstallTimeout) resp, err := client.Get(source) if err != nil { return fmt.Errorf("failed to download: %w", err) diff --git a/internal/httpclient/httpclient.go b/internal/httpclient/httpclient.go new file mode 100644 index 0000000000000000000000000000000000000000..d377d6bda47cdf0b80839b837b3ffe998d1db3ef --- /dev/null +++ b/internal/httpclient/httpclient.go @@ -0,0 +1,45 @@ +// Package httpclient centralizes HTTP timeout defaults so the rest of the +// codebase doesn't sprinkle magic numbers across packages. +package httpclient + +import ( + "fmt" + "net/http" + "time" +) + +// Named timeouts. Each constant documents the call site it covers so +// future contributors don't have to grep for callers. +const ( + // PluginCallTimeout bounds Lua-driven plugin HTTP calls (plugin/http.go). + PluginCallTimeout = 10 * time.Second + // RegistryFetchTimeout bounds plugin registry / plugin file fetches (plugins/embed.go). + RegistryFetchTimeout = 10 * time.Second + // RemoteImageTimeout bounds inline image fetches (view/html.go). + // Kept short so message rendering doesn't stall. + RemoteImageTimeout = 5 * time.Second + // InstallTimeout bounds CLI install downloads (cli/install.go). + InstallTimeout = 30 * time.Second + // UpdateCheckTimeout bounds version checks and asset downloads from main (main.go). + UpdateCheckTimeout = 30 * time.Second +) + +// New returns an http.Client preconfigured with the given timeout. +func New(timeout time.Duration) *http.Client { + return &http.Client{Timeout: timeout} +} + +// NewWithRedirectCap returns an http.Client with the given timeout and a +// hard cap on the number of redirects it will follow before giving up. +// Used by the main update / asset download client to avoid infinite chains. +func NewWithRedirectCap(timeout time.Duration, maxRedirects int) *http.Client { + return &http.Client{ + Timeout: timeout, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= maxRedirects { + return fmt.Errorf("stopped after %d redirects", maxRedirects) + } + return nil + }, + } +} diff --git a/internal/httpclient/httpclient_test.go b/internal/httpclient/httpclient_test.go new file mode 100644 index 0000000000000000000000000000000000000000..510c40e972ee394a03b0225a664a0d954786aa23 --- /dev/null +++ b/internal/httpclient/httpclient_test.go @@ -0,0 +1,83 @@ +package httpclient + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +func TestTimeoutConstants(t *testing.T) { + cases := []struct { + name string + got time.Duration + min time.Duration + }{ + {"PluginCallTimeout", PluginCallTimeout, time.Second}, + {"RegistryFetchTimeout", RegistryFetchTimeout, time.Second}, + {"RemoteImageTimeout", RemoteImageTimeout, time.Second}, + {"InstallTimeout", InstallTimeout, time.Second}, + {"UpdateCheckTimeout", UpdateCheckTimeout, time.Second}, + } + for _, c := range cases { + if c.got < c.min { + t.Errorf("%s = %s, want at least %s", c.name, c.got, c.min) + } + } +} + +func TestNew_AppliesTimeout(t *testing.T) { + c := New(7 * time.Second) + if c.Timeout != 7*time.Second { + t.Errorf("New(7s).Timeout = %s, want 7s", c.Timeout) + } +} + +func TestNewWithRedirectCap_AppliesTimeoutAndRedirects(t *testing.T) { + c := NewWithRedirectCap(11*time.Second, 3) + if c.Timeout != 11*time.Second { + t.Errorf("Timeout = %s, want 11s", c.Timeout) + } + if c.CheckRedirect == nil { + t.Fatal("CheckRedirect is nil; want a redirect-cap function") + } + + // Build a stubbed redirect chain and verify the cap fires at the + // configured maxRedirects. + req, _ := http.NewRequest(http.MethodGet, "http://example.invalid/", nil) + via := []*http.Request{} + for i := 0; i < 3; i++ { + if err := c.CheckRedirect(req, via); err != nil { + t.Fatalf("CheckRedirect rejected %d-redirect chain: %v", i, err) + } + via = append(via, req) + } + if err := c.CheckRedirect(req, via); err == nil { + t.Error("CheckRedirect(via len=3) returned nil; want stopped error") + } else if !strings.Contains(err.Error(), "stopped after 3 redirects") { + t.Errorf("CheckRedirect error = %q, want 'stopped after 3 redirects' substring", err.Error()) + } +} + +// TestNewWithRedirectCap_LiveServer is a defense-in-depth integration check +// that the redirect cap is actually honored by net/http when wired up. It +// uses an in-process server so it stays hermetic. +func TestNewWithRedirectCap_LiveServer(t *testing.T) { + hops := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hops++ + http.Redirect(w, r, "/next", http.StatusFound) + })) + defer server.Close() + + c := NewWithRedirectCap(2*time.Second, 2) + resp, err := c.Get(server.URL + "/start") + if err == nil { + resp.Body.Close() + t.Fatal("expected redirect-cap error, got nil") + } + if !strings.Contains(err.Error(), "stopped after 2 redirects") { + t.Errorf("redirect error = %v, want substring 'stopped after 2 redirects'", err) + } +} diff --git a/main.go b/main.go index 33d329e1690511abf16131f085b1f315f3f3d2fb..0af44c8f81838222227d7df38a81a11d5419637f 100644 --- a/main.go +++ b/main.go @@ -11,7 +11,6 @@ import ( "fmt" "io" "log" - "net/http" "net/url" "os" "os/exec" @@ -39,6 +38,7 @@ import ( "github.com/floatpane/matcha/fetcher" "github.com/floatpane/matcha/i18n" _ "github.com/floatpane/matcha/i18n/languages" + "github.com/floatpane/matcha/internal/httpclient" "github.com/floatpane/matcha/notify" "github.com/floatpane/matcha/plugin" "github.com/floatpane/matcha/sender" @@ -62,16 +62,7 @@ var ( date = "" // httpClient is used for all outbound HTTP requests (update checks, asset downloads). - // Configured with a 30s timeout to prevent indefinite hangs on slow/unresponsive servers. - httpClient = &http.Client{ - Timeout: 30 * time.Second, - CheckRedirect: func(req *http.Request, via []*http.Request) error { - if len(via) >= 5 { - return fmt.Errorf("stopped after 5 redirects") - } - return nil - }, - } + httpClient = httpclient.NewWithRedirectCap(httpclient.UpdateCheckTimeout, 5) ) // UpdateAvailableMsg is sent into the TUI when a newer release is detected. diff --git a/plugin/http.go b/plugin/http.go index a608d164d17a74fbf8b5d88f536f51d0710f3e5a..b0eb2738b58af5caa63dab85017dd51b7b6b3a2b 100644 --- a/plugin/http.go +++ b/plugin/http.go @@ -4,19 +4,15 @@ import ( "io" "net/http" "strings" - "time" lua "github.com/yuin/gopher-lua" -) -const ( - httpTimeout = 10 * time.Second - httpMaxBodySize = 1 << 20 // 1 MB + "github.com/floatpane/matcha/internal/httpclient" ) -var httpClient = &http.Client{ - Timeout: httpTimeout, -} +const httpMaxBodySize = 1 << 20 // 1 MB + +var httpClient = httpclient.New(httpclient.PluginCallTimeout) // luaHTTP implements matcha.http(options) — make an HTTP request. // diff --git a/plugins/embed.go b/plugins/embed.go index 2fb8e6abfe2a8740817cf17d5ba10ea59bd03418..084e121db18e62420d46d606594d30043cacd0fc 100644 --- a/plugins/embed.go +++ b/plugins/embed.go @@ -5,7 +5,8 @@ import ( "fmt" "io" "net/http" - "time" + + "github.com/floatpane/matcha/internal/httpclient" ) const RegistryURL = "https://raw.githubusercontent.com/floatpane/matcha/master/plugins/registry.json" @@ -22,7 +23,7 @@ type PluginEntry struct { // FetchRegistry fetches the plugin registry from GitHub. func FetchRegistry() ([]PluginEntry, error) { - client := &http.Client{Timeout: 10 * time.Second} + client := httpclient.New(httpclient.RegistryFetchTimeout) resp, err := client.Get(RegistryURL) if err != nil { return nil, fmt.Errorf("failed to fetch registry: %w", err) @@ -53,7 +54,7 @@ func FetchPlugin(entry PluginEntry) ([]byte, error) { url = RawPluginBaseURL + entry.File } - client := &http.Client{Timeout: 10 * time.Second} + client := httpclient.New(httpclient.RegistryFetchTimeout) resp, err := client.Get(url) if err != nil { return nil, fmt.Errorf("failed to fetch plugin: %w", err) diff --git a/view/html.go b/view/html.go index c3ca1692a1bf9a1918f70d4895871f0a4865fbdc..186cd036190283a61932cdfca15632f02c09c7f6 100644 --- a/view/html.go +++ b/view/html.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "mime/quotedprintable" - "net/http" "os" "regexp" "strings" @@ -14,6 +13,7 @@ import ( "charm.land/lipgloss/v2" "github.com/floatpane/matcha/clib" + "github.com/floatpane/matcha/internal/httpclient" "github.com/floatpane/matcha/theme" lru "github.com/hashicorp/golang-lru/v2" ) @@ -297,7 +297,7 @@ func fetchRemoteBase64(url string) string { return cached } - client := &http.Client{Timeout: 5 * time.Second} + client := httpclient.New(httpclient.RemoteImageTimeout) resp, err := client.Get(url) if err != nil { debugImageProtocol("remote fetch failed url=%s err=%v", url, err)