diff --git a/pkg/htmltext/htmltext.go b/pkg/htmltext/htmltext.go index 929080838..47daeef40 100644 --- a/pkg/htmltext/htmltext.go +++ b/pkg/htmltext/htmltext.go @@ -26,6 +26,7 @@ import ( "regexp" "strings" "sync/atomic" + "time" "unicode" "unicode/utf8" @@ -238,15 +239,27 @@ func FetchMatchedExcerpt(html string, words []string, trimMarker string, trimLen return FetchRangedExcerpt(html, trimMarker, runeOffset, runeLimit) } +// fetchedPicMaxBytes caps how much of a remote image is read, so a hostile or +// misconfigured URL cannot exhaust memory. +const fetchedPicMaxBytes = 5 << 20 + +// picClient is used for anonymous, user-influenced outbound fetches. The zero +// value of http.Client has no timeout, so a slow or blackholed host would block +// the handler goroutine until the OS gives up. +var picClient = &http.Client{Timeout: 15 * time.Second} + func GetPicByUrl(url string) string { - res, err := http.Get(url) + res, err := picClient.Get(url) if err != nil { return "" } defer func() { _ = res.Body.Close() }() - pix, err := io.ReadAll(res.Body) + if res.StatusCode != http.StatusOK { + return "" + } + pix, err := io.ReadAll(io.LimitReader(res.Body, fetchedPicMaxBytes)) if err != nil { return "" } diff --git a/pkg/htmltext/htmltext_test.go b/pkg/htmltext/htmltext_test.go index bcedcb3c8..781d5ff12 100644 --- a/pkg/htmltext/htmltext_test.go +++ b/pkg/htmltext/htmltext_test.go @@ -21,8 +21,11 @@ package htmltext import ( "fmt" + "net/http" + "net/http/httptest" "strings" "testing" + "time" "github.com/stretchr/testify/assert" ) @@ -331,3 +334,56 @@ func TestFetchMatchedExcerpt(t *testing.T) { actual = FetchMatchedExcerpt(html, []string{"δΈ­ζ–‡", "πŸ˜‚"}, "...", 6) assert.Equal(t, expected, actual) } + +func TestGetPicByUrlReturnsBody(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("image-bytes")) + })) + defer srv.Close() + + assert.Equal(t, "image-bytes", GetPicByUrl(srv.URL)) +} + +func TestGetPicByUrlReturnsEmptyOnErrorStatus(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte("not found")) + })) + defer srv.Close() + + assert.Equal(t, "", GetPicByUrl(srv.URL)) +} + +// A slow origin must not pin the handler goroutine indefinitely: /favicon.ico +// calls GetPicByUrl on every anonymous request, so an unresponsive branding URL +// would otherwise accumulate stuck goroutines with no upper bound. +func TestGetPicByUrlTimesOut(t *testing.T) { + release := make(chan struct{}) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + <-release + })) + defer func() { + close(release) + srv.Close() + }() + + original := picClient.Timeout + picClient.Timeout = 100 * time.Millisecond + defer func() { picClient.Timeout = original }() + + done := make(chan string, 1) + go func() { done <- GetPicByUrl(srv.URL) }() + + select { + case got := <-done: + assert.Equal(t, "", got, "a timed-out fetch must yield an empty result") + case <-time.After(10 * time.Second): + t.Fatal("GetPicByUrl did not honour the client timeout") + } +} + +func TestPicClientHasTimeout(t *testing.T) { + // Guards against regressing to a bare &http.Client{}, which has no timeout. + assert.NotZero(t, picClient.Timeout) +}