From ab61b8589ac7b897d6cfc036267321ed52cd809b Mon Sep 17 00:00:00 2001 From: SiriusTL666 <214846386+SiriusTL666@users.noreply.github.com> Date: Sun, 25 Oct 2026 21:26:07 +0800 Subject: [PATCH 1/2] fix(branding): bound the favicon fetch timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GetPicByUrl used http.Get, which goes through http.DefaultClient. The zero value of http.Client has no timeout, so a branding URL pointing at a host that is slow or blackholed blocks the handler goroutine until the OS gives up. /favicon.ico calls this for every anonymous request, so an unreachable favicon or square icon URL accumulates stuck goroutines and connections with no upper bound β€” a cheap way to exhaust the server from the outside. - Use a package-level client with a 15s timeout, matching the existing remoteVersion fetch in dashboard_service. - Treat non-200 responses as empty instead of returning the error body as if it were image data. - Cap the read at 5MB so a misconfigured or hostile URL cannot exhaust memory. - Add tests covering success, error status, the timeout, and that the client keeps a non-zero timeout. --- pkg/htmltext/htmltext.go | 17 +++++++++-- pkg/htmltext/htmltext_test.go | 56 +++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) 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) +} From b3e107e2f561de577ffd72fc9a0ece7a10d53592 Mon Sep 17 00:00:00 2001 From: SiriusTL666 <214846386+SiriusTL666@users.noreply.github.com> Date: Fri, 6 Nov 2026 13:20:30 +0800 Subject: [PATCH 2/2] fix(branding): reject oversized favicon responses instead of truncating io.LimitReader silently truncates a body larger than the cap, so GetPicByUrl returned incomplete bytes as a successful image. Read up to fetchedPicMaxBytes+1 and return an empty result when the limit is exceeded. Add tests for the at-limit (accepted) and over-limit (rejected) cases. --- pkg/htmltext/htmltext.go | 7 ++++++- pkg/htmltext/htmltext_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/pkg/htmltext/htmltext.go b/pkg/htmltext/htmltext.go index 47daeef40..a6e9d64c7 100644 --- a/pkg/htmltext/htmltext.go +++ b/pkg/htmltext/htmltext.go @@ -259,9 +259,14 @@ func GetPicByUrl(url string) string { if res.StatusCode != http.StatusOK { return "" } - pix, err := io.ReadAll(io.LimitReader(res.Body, fetchedPicMaxBytes)) + // Read one byte past the cap: LimitReader alone would silently truncate an + // oversized body and hand back a corrupt image as if the fetch succeeded. + pix, err := io.ReadAll(io.LimitReader(res.Body, fetchedPicMaxBytes+1)) if err != nil { return "" } + if len(pix) > fetchedPicMaxBytes { + return "" + } return string(pix) } diff --git a/pkg/htmltext/htmltext_test.go b/pkg/htmltext/htmltext_test.go index 781d5ff12..da32956e2 100644 --- a/pkg/htmltext/htmltext_test.go +++ b/pkg/htmltext/htmltext_test.go @@ -20,6 +20,7 @@ package htmltext import ( + "bytes" "fmt" "net/http" "net/http/httptest" @@ -383,6 +384,32 @@ func TestGetPicByUrlTimesOut(t *testing.T) { } } +// A body at the cap is still a complete image and must be served. +func TestGetPicByUrlAcceptsBodyAtSizeLimit(t *testing.T) { + body := bytes.Repeat([]byte("a"), fetchedPicMaxBytes) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write(body) + })) + defer srv.Close() + + got := GetPicByUrl(srv.URL) + assert.Len(t, got, fetchedPicMaxBytes) +} + +// One byte past the cap means the response was truncated, so the bytes are not +// a usable image. Handing them back as a success would serve a corrupt favicon. +func TestGetPicByUrlRejectsBodyOverSizeLimit(t *testing.T) { + body := bytes.Repeat([]byte("a"), fetchedPicMaxBytes+1) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write(body) + })) + defer srv.Close() + + assert.Equal(t, "", GetPicByUrl(srv.URL)) +} + func TestPicClientHasTimeout(t *testing.T) { // Guards against regressing to a bare &http.Client{}, which has no timeout. assert.NotZero(t, picClient.Timeout)