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