From add5503a7f80780ab7291bb7a099b8136083e9a3 Mon Sep 17 00:00:00 2001 From: Bunlong Heng Date: Thu, 6 Aug 2026 10:07:39 -0400 Subject: [PATCH] fix: add timeout to remote HTTP fetches Remote sources (github://, gitlab:// and https:// URLs) were fetched with http.Get using the default client, which has no timeout. A slow or unresponsive host could make glow hang indefinitely. Introduce a shared http.Client with a 30s timeout and route all outbound requests through it. Add a test that verifies the client has a positive timeout and that fetching from an unresponsive server returns promptly instead of blocking. --- github.go | 4 ++-- gitlab.go | 4 ++-- httpclient.go | 17 +++++++++++++++++ httpclient_test.go | 39 +++++++++++++++++++++++++++++++++++++++ main.go | 2 +- 5 files changed, 61 insertions(+), 5 deletions(-) create mode 100644 httpclient.go create mode 100644 httpclient_test.go diff --git a/github.go b/github.go index fe862e3..6d35ee2 100644 --- a/github.go +++ b/github.go @@ -25,7 +25,7 @@ func findGitHubREADME(u *url.URL) (*source, error) { //nolint:bodyclose // it is closed on the caller - res, err := http.Get(apiURL) //nolint: gosec,noctx + res, err := httpClient.Get(apiURL) //nolint: gosec,noctx if err != nil { return nil, fmt.Errorf("unable to get url: %w", err) } @@ -43,7 +43,7 @@ func findGitHubREADME(u *url.URL) (*source, error) { if res.StatusCode == http.StatusOK { //nolint:bodyclose // it is closed on the caller - resp, err := http.Get(result.DownloadURL) //nolint: noctx + resp, err := httpClient.Get(result.DownloadURL) //nolint: noctx if err != nil { return nil, fmt.Errorf("unable to get url: %w", err) } diff --git a/gitlab.go b/gitlab.go index 68256be..c1184e5 100644 --- a/gitlab.go +++ b/gitlab.go @@ -27,7 +27,7 @@ func findGitLabREADME(u *url.URL) (*source, error) { //nolint:bodyclose // it is closed on the caller - res, err := http.Get(apiURL) //nolint: gosec,noctx + res, err := httpClient.Get(apiURL) //nolint: gosec,noctx if err != nil { return nil, fmt.Errorf("unable to get url: %w", err) } @@ -47,7 +47,7 @@ func findGitLabREADME(u *url.URL) (*source, error) { if res.StatusCode == http.StatusOK { //nolint:bodyclose // it is closed on the caller - resp, err := http.Get(readmeRawURL) //nolint: gosec,noctx + resp, err := httpClient.Get(readmeRawURL) //nolint: gosec,noctx if err != nil { return nil, fmt.Errorf("unable to get url: %w", err) } diff --git a/httpclient.go b/httpclient.go new file mode 100644 index 0000000..17ead51 --- /dev/null +++ b/httpclient.go @@ -0,0 +1,17 @@ +package main + +import ( + "net/http" + "time" +) + +// httpRequestTimeout bounds how long glow will wait on a single remote fetch. +// Without it, a slow or unresponsive host (for example when resolving a +// github://, gitlab:// or https:// source) could make glow hang indefinitely. +const httpRequestTimeout = 30 * time.Second + +// httpClient is the shared HTTP client used for all outbound requests. It sets +// a timeout so that an unresponsive remote host cannot stall glow forever. +var httpClient = &http.Client{ + Timeout: httpRequestTimeout, +} diff --git a/httpclient_test.go b/httpclient_test.go new file mode 100644 index 0000000..26f461c --- /dev/null +++ b/httpclient_test.go @@ -0,0 +1,39 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "testing" + "time" +) + +// TestHTTPClientHasTimeout guards against regressing back to a client with no +// timeout, which would let an unresponsive host hang glow indefinitely. +func TestHTTPClientHasTimeout(t *testing.T) { + if httpClient.Timeout <= 0 { + t.Fatalf("httpClient must have a positive timeout, got %v", httpClient.Timeout) + } +} + +// TestSourceFromArgHTTPTimeout verifies that fetching a remote source honors +// the client timeout instead of blocking forever on an unresponsive host. +func TestSourceFromArgHTTPTimeout(t *testing.T) { + block := make(chan struct{}) + srv := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + <-block // never respond until the test tears the server down + })) + defer srv.Close() + defer close(block) // runs first (LIFO): unblock handler before srv.Close() + + orig := httpClient.Timeout + httpClient.Timeout = 100 * time.Millisecond + defer func() { httpClient.Timeout = orig }() + + start := time.Now() + if _, err := sourceFromArg(srv.URL); err == nil { + t.Fatal("expected a timeout error, got nil") + } + if elapsed := time.Since(start); elapsed > 5*time.Second { + t.Fatalf("request did not honor timeout, took %s", elapsed) + } +} diff --git a/main.go b/main.go index b31ca15..287db02 100644 --- a/main.go +++ b/main.go @@ -92,7 +92,7 @@ func sourceFromArg(arg string) (*source, error) { return nil, fmt.Errorf("%s is not a supported protocol", u.Scheme) } // consumer of the source is responsible for closing the ReadCloser. - resp, err := http.Get(u.String()) //nolint: noctx,bodyclose + resp, err := httpClient.Get(u.String()) //nolint: noctx,bodyclose if err != nil { return nil, fmt.Errorf("unable to get url: %w", err) }