Merge add5503a7f into e5cb757278

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)
 		}

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)
 		}

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,
+}

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)
+	}
+}

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)
 			}