detached aed3fd6

Merge pull request #277 from bnema/fix/favicon-reject-placeholder-conversion

fix(favicon): reject placeholder conversions
commit
aed3fd671b99c2bc000f26be40eb7b8ff685985e
author
bnema · · 2026-06-08 21:06 UTC
committer
GitHub · · 2026-06-08 21:06 UTC
merge of
1b29d2c + f25be77 · diff against the first parent
signature
GPG UnverifiedB5690EEEBB952194 The key is unknown
3 files · +69 -62
Split
UnifiedSplit
M internal/application/usecase/favicon_test.go +50 -6
@@ -238,6 +238,36 @@ func TestFaviconRefreshFromIconURLsUsesPageURLKey(t *testing.T) {
238238 }
239239 }
240240
241+func TestFaviconRefreshFromIconURLsSkipsUnsupportedCandidates(t *testing.T) {
242+ fx := newFaviconFixture()
243+ fx.fetcher.responses = map[string]fakeFetchedIcon{
244+ "https://cdn.example.net/icon.svg": {bytes: []byte("svg"), contentType: "image/svg+xml"},
245+ "https://cdn.example.net/icon.png": {bytes: []byte("png"), contentType: "image/png"},
246+ }
247+ fx.converter.errByContentType = map[string]error{"image/svg+xml": ErrFaviconMiss}
248+
249+ err := fx.uc.RefreshFromIconURLs(context.Background(), "https://docs.example.com/page", []string{
250+ "https://cdn.example.net/icon.svg",
251+ "https://cdn.example.net/icon.png",
252+ })
253+ if err != nil {
254+ t.Fatal(err)
255+ }
256+ if fx.fetcher.calls != 2 {
257+ t.Fatalf("fetcher calls=%d, want 2", fx.fetcher.calls)
258+ }
259+ meta := fx.repo.byKey["docs.example.com"]
260+ if meta == nil {
261+ t.Fatalf("expected page URL key to be stored; keys=%v", fx.repo.byKey)
262+ }
263+ if meta.SourceURL != "https://cdn.example.net/icon.png" || meta.ContentType != "image/png" {
264+ t.Fatalf("stored meta = %+v, want png candidate", meta)
265+ }
266+ if string(fx.blobs.original["docs.example.com"]) != "png" {
267+ t.Fatalf("stored original = %q, want png", fx.blobs.original["docs.example.com"])
268+ }
269+}
270+
241271 func TestFaviconObserveConversionErrorDoesNotMutateExistingAssets(t *testing.T) {
242272 fx := newFaviconFixture()
243273 fx.seedOriginal([]byte("old"))
@@ -462,15 +492,19 @@ func (b *fakeBlobStore) RemoveDerived(_ context.Context, key favicon.Key) error
462492 }
463493
464494 type fakeConverter struct {
465- calls int
466- err error
495+ calls int
496+ err error
497+ errByContentType map[string]error
467498 }
468499
469-func (c *fakeConverter) Convert(_ context.Context, original []byte, _ string, sizes []int) (*appport.ConvertedFavicon, error) {
500+func (c *fakeConverter) Convert(_ context.Context, original []byte, contentType string, sizes []int) (*appport.ConvertedFavicon, error) {
470501 c.calls++
471502 if c.err != nil {
472503 return nil, c.err
473504 }
505+ if err := c.errByContentType[contentType]; err != nil {
506+ return nil, err
507+ }
474508 out := &appport.ConvertedFavicon{PNG: append([]byte("png:"), original...), SizedPNG: map[int][]byte{}}
475509 for _, s := range sizes {
476510 out.SizedPNG[s] = append([]byte("png32:"), original...)
@@ -514,9 +548,15 @@ func (i *fakeInvalidators) InvalidateAll(_ context.Context, _ favicon.Key) error
514548 return nil
515549 }
516550
551+type fakeFetchedIcon struct {
552+ bytes []byte
553+ contentType string
554+}
555+
517556 type fakeFetcher struct {
518- calls int
519- err error
557+ calls int
558+ err error
559+ responses map[string]fakeFetchedIcon
520560 }
521561
522562 func (f *fakeFetcher) Fetch(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) {
@@ -528,5 +568,9 @@ func (f *fakeFetcher) Fetch(_ context.Context, req appport.FaviconFetchRequest)
528568 if iconURL == "" {
529569 iconURL = "https://example.com/favicon.ico"
530570 }
531- return &appport.FaviconFetchedIcon{PageURL: req.PageURL, IconURL: iconURL, Bytes: []byte("fetched"), Source: favicon.SourceDuckDuckGo, ContentType: "image/png"}, nil
571+ response := f.responses[iconURL]
572+ if response.bytes == nil {
573+ response = fakeFetchedIcon{bytes: []byte("fetched"), contentType: "image/png"}
574+ }
575+ return &appport.FaviconFetchedIcon{PageURL: req.PageURL, IconURL: iconURL, Bytes: response.bytes, Source: favicon.SourceDuckDuckGo, ContentType: response.contentType}, nil
532576 }
@@ -238,6 +238,36 @@ func TestFaviconRefreshFromIconURLsUsesPageURLKey(t *testing.T) {
238 }238 }
239 }239 }
240 240
241+func TestFaviconRefreshFromIconURLsSkipsUnsupportedCandidates(t *testing.T) {
242+ fx := newFaviconFixture()
243+ fx.fetcher.responses = map[string]fakeFetchedIcon{
244+ "https://cdn.example.net/icon.svg": {bytes: []byte("svg"), contentType: "image/svg+xml"},
245+ "https://cdn.example.net/icon.png": {bytes: []byte("png"), contentType: "image/png"},
246+ }
247+ fx.converter.errByContentType = map[string]error{"image/svg+xml": ErrFaviconMiss}
248+
249+ err := fx.uc.RefreshFromIconURLs(context.Background(), "https://docs.example.com/page", []string{
250+ "https://cdn.example.net/icon.svg",
251+ "https://cdn.example.net/icon.png",
252+ })
253+ if err != nil {
254+ t.Fatal(err)
255+ }
256+ if fx.fetcher.calls != 2 {
257+ t.Fatalf("fetcher calls=%d, want 2", fx.fetcher.calls)
258+ }
259+ meta := fx.repo.byKey["docs.example.com"]
260+ if meta == nil {
261+ t.Fatalf("expected page URL key to be stored; keys=%v", fx.repo.byKey)
262+ }
263+ if meta.SourceURL != "https://cdn.example.net/icon.png" || meta.ContentType != "image/png" {
264+ t.Fatalf("stored meta = %+v, want png candidate", meta)
265+ }
266+ if string(fx.blobs.original["docs.example.com"]) != "png" {
267+ t.Fatalf("stored original = %q, want png", fx.blobs.original["docs.example.com"])
268+ }
269+}
270+
241 func TestFaviconObserveConversionErrorDoesNotMutateExistingAssets(t *testing.T) {271 func TestFaviconObserveConversionErrorDoesNotMutateExistingAssets(t *testing.T) {
242 fx := newFaviconFixture()272 fx := newFaviconFixture()
243 fx.seedOriginal([]byte("old"))273 fx.seedOriginal([]byte("old"))
@@ -462,15 +492,19 @@ func (b *fakeBlobStore) RemoveDerived(_ context.Context, key favicon.Key) error
462 }492 }
463 493
464 type fakeConverter struct {494 type fakeConverter struct {
465- calls int495+ calls int
466- err error496+ err error
497+ errByContentType map[string]error
467 }498 }
468 499
469-func (c *fakeConverter) Convert(_ context.Context, original []byte, _ string, sizes []int) (*appport.ConvertedFavicon, error) {500+func (c *fakeConverter) Convert(_ context.Context, original []byte, contentType string, sizes []int) (*appport.ConvertedFavicon, error) {
470 c.calls++501 c.calls++
471 if c.err != nil {502 if c.err != nil {
472 return nil, c.err503 return nil, c.err
473 }504 }
505+ if err := c.errByContentType[contentType]; err != nil {
506+ return nil, err
507+ }
474 out := &appport.ConvertedFavicon{PNG: append([]byte("png:"), original...), SizedPNG: map[int][]byte{}}508 out := &appport.ConvertedFavicon{PNG: append([]byte("png:"), original...), SizedPNG: map[int][]byte{}}
475 for _, s := range sizes {509 for _, s := range sizes {
476 out.SizedPNG[s] = append([]byte("png32:"), original...)510 out.SizedPNG[s] = append([]byte("png32:"), original...)
@@ -514,9 +548,15 @@ func (i *fakeInvalidators) InvalidateAll(_ context.Context, _ favicon.Key) error
514 return nil548 return nil
515 }549 }
516 550
551+type fakeFetchedIcon struct {
552+ bytes []byte
553+ contentType string
554+}
555+
517 type fakeFetcher struct {556 type fakeFetcher struct {
518- calls int557+ calls int
519- err error558+ err error
559+ responses map[string]fakeFetchedIcon
520 }560 }
521 561
522 func (f *fakeFetcher) Fetch(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) {562 func (f *fakeFetcher) Fetch(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) {
@@ -528,5 +568,9 @@ func (f *fakeFetcher) Fetch(_ context.Context, req appport.FaviconFetchRequest)
528 if iconURL == "" {568 if iconURL == "" {
529 iconURL = "https://example.com/favicon.ico"569 iconURL = "https://example.com/favicon.ico"
530 }570 }
531- return &appport.FaviconFetchedIcon{PageURL: req.PageURL, IconURL: iconURL, Bytes: []byte("fetched"), Source: favicon.SourceDuckDuckGo, ContentType: "image/png"}, nil571+ response := f.responses[iconURL]
572+ if response.bytes == nil {
573+ response = fakeFetchedIcon{bytes: []byte("fetched"), contentType: "image/png"}
574+ }
575+ return &appport.FaviconFetchedIcon{PageURL: req.PageURL, IconURL: iconURL, Bytes: response.bytes, Source: favicon.SourceDuckDuckGo, ContentType: response.contentType}, nil
532 }576 }
M internal/infrastructure/favicon/converter.go +2 -54
@@ -9,8 +9,6 @@ import (
99 "image/color"
1010 "image/png"
1111 "mime"
12- "regexp"
13- "strconv"
1412 "strings"
1513
1614 appport "github.com/bnema/dumber/internal/application/port"
@@ -53,7 +51,7 @@ func (*ImageConverter) Convert(ctx context.Context, original []byte, contentType
5351 func decodeFaviconImage(data []byte, ct string) (image.Image, error) {
5452 switch ct {
5553 case "image/svg+xml":
56- return renderSimpleSVG(data)
54+ return nil, fmt.Errorf("%w: svg favicon conversion is disabled", appport.ErrFaviconMiss)
5755 case "image/x-icon", "image/vnd.microsoft.icon", "image/ico":
5856 img, err := decodeICO(data)
5957 if err != nil {
@@ -61,7 +59,7 @@ func decodeFaviconImage(data []byte, ct string) (image.Image, error) {
6159 }
6260 return img, nil
6361 case "image/webp":
64- return renderWEBPPlaceholder(data)
62+ return nil, fmt.Errorf("%w: webp favicon conversion is disabled", appport.ErrFaviconMiss)
6563 }
6664 if ct != "" && ct != "image/png" && ct != "image/jpeg" && ct != "image/gif" && ct != "image/webp" {
6765 return nil, fmt.Errorf("%w: unsupported favicon content type %s", appport.ErrFaviconMiss, ct)
@@ -209,53 +207,3 @@ func decodeICODIB(data []byte, bitCountHint int) (image.Image, error) {
209207 }
210208 return img, nil
211209 }
212-
213-var svgSizeRe = regexp.MustCompile(`(?i)\b(width|height)=["']?(\d+)`) // intentionally small safe subset
214-var svgFillRe = regexp.MustCompile(`(?i)\bfill=["']#?([0-9a-f]{6})`)
215-
216-func renderWEBPPlaceholder(data []byte) (image.Image, error) {
217- if len(data) < 12 || !bytes.HasPrefix(data, []byte("RIFF")) || string(data[8:12]) != "WEBP" {
218- return nil, fmt.Errorf("%w: malformed webp", appport.ErrFaviconMiss)
219- }
220- return solidImage(64, color.RGBA{R: 128, G: 128, B: 128, A: 255}), nil
221-}
222-
223-func renderSimpleSVG(data []byte) (image.Image, error) {
224- trimmed := bytes.TrimSpace(data)
225- if !bytes.Contains(trimmed, []byte("<svg")) {
226- return nil, fmt.Errorf("%w: malformed svg", appport.ErrFaviconMiss)
227- }
228- size := 64
229- for _, m := range svgSizeRe.FindAllSubmatch(trimmed, -1) {
230- if v, err := strconv.Atoi(string(m[2])); err == nil && v > 0 && v <= 1024 {
231- size = v
232- break
233- }
234- }
235- fill := color.RGBA{R: 128, G: 128, B: 128, A: 255}
236- if m := svgFillRe.FindSubmatch(trimmed); len(m) == 2 {
237- if parsed, ok := parseHexColor(string(m[1])); ok {
238- fill = parsed
239- }
240- }
241- return solidImage(size, fill), nil
242-}
243-
244-func solidImage(size int, fill color.RGBA) image.Image {
245- img := image.NewRGBA(image.Rect(0, 0, size, size))
246- for y := 0; y < size; y++ {
247- for x := 0; x < size; x++ {
248- img.SetRGBA(x, y, fill)
249- }
250- }
251- return img
252-}
253-
254-func parseHexColor(hex string) (color.RGBA, bool) {
255- v, err := strconv.ParseUint(hex, 16, 32)
256- if err != nil {
257- return color.RGBA{}, false
258- }
259- const greenShift = 8
260- return color.RGBA{R: uint8(v >> 16), G: uint8(v >> greenShift), B: uint8(v), A: 255}, true
261-}
@@ -9,8 +9,6 @@ import (
9 "image/color"9 "image/color"
10 "image/png"10 "image/png"
11 "mime"11 "mime"
12- "regexp"
13- "strconv"
14 "strings"12 "strings"
15 13
16 appport "github.com/bnema/dumber/internal/application/port"14 appport "github.com/bnema/dumber/internal/application/port"
@@ -53,7 +51,7 @@ func (*ImageConverter) Convert(ctx context.Context, original []byte, contentType
53 func decodeFaviconImage(data []byte, ct string) (image.Image, error) {51 func decodeFaviconImage(data []byte, ct string) (image.Image, error) {
54 switch ct {52 switch ct {
55 case "image/svg+xml":53 case "image/svg+xml":
56- return renderSimpleSVG(data)54+ return nil, fmt.Errorf("%w: svg favicon conversion is disabled", appport.ErrFaviconMiss)
57 case "image/x-icon", "image/vnd.microsoft.icon", "image/ico":55 case "image/x-icon", "image/vnd.microsoft.icon", "image/ico":
58 img, err := decodeICO(data)56 img, err := decodeICO(data)
59 if err != nil {57 if err != nil {
@@ -61,7 +59,7 @@ func decodeFaviconImage(data []byte, ct string) (image.Image, error) {
61 }59 }
62 return img, nil60 return img, nil
63 case "image/webp":61 case "image/webp":
64- return renderWEBPPlaceholder(data)62+ return nil, fmt.Errorf("%w: webp favicon conversion is disabled", appport.ErrFaviconMiss)
65 }63 }
66 if ct != "" && ct != "image/png" && ct != "image/jpeg" && ct != "image/gif" && ct != "image/webp" {64 if ct != "" && ct != "image/png" && ct != "image/jpeg" && ct != "image/gif" && ct != "image/webp" {
67 return nil, fmt.Errorf("%w: unsupported favicon content type %s", appport.ErrFaviconMiss, ct)65 return nil, fmt.Errorf("%w: unsupported favicon content type %s", appport.ErrFaviconMiss, ct)
@@ -209,53 +207,3 @@ func decodeICODIB(data []byte, bitCountHint int) (image.Image, error) {
209 }207 }
210 return img, nil208 return img, nil
211 }209 }
212-
213-var svgSizeRe = regexp.MustCompile(`(?i)\b(width|height)=["']?(\d+)`) // intentionally small safe subset
214-var svgFillRe = regexp.MustCompile(`(?i)\bfill=["']#?([0-9a-f]{6})`)
215-
216-func renderWEBPPlaceholder(data []byte) (image.Image, error) {
217- if len(data) < 12 || !bytes.HasPrefix(data, []byte("RIFF")) || string(data[8:12]) != "WEBP" {
218- return nil, fmt.Errorf("%w: malformed webp", appport.ErrFaviconMiss)
219- }
220- return solidImage(64, color.RGBA{R: 128, G: 128, B: 128, A: 255}), nil
221-}
222-
223-func renderSimpleSVG(data []byte) (image.Image, error) {
224- trimmed := bytes.TrimSpace(data)
225- if !bytes.Contains(trimmed, []byte("<svg")) {
226- return nil, fmt.Errorf("%w: malformed svg", appport.ErrFaviconMiss)
227- }
228- size := 64
229- for _, m := range svgSizeRe.FindAllSubmatch(trimmed, -1) {
230- if v, err := strconv.Atoi(string(m[2])); err == nil && v > 0 && v <= 1024 {
231- size = v
232- break
233- }
234- }
235- fill := color.RGBA{R: 128, G: 128, B: 128, A: 255}
236- if m := svgFillRe.FindSubmatch(trimmed); len(m) == 2 {
237- if parsed, ok := parseHexColor(string(m[1])); ok {
238- fill = parsed
239- }
240- }
241- return solidImage(size, fill), nil
242-}
243-
244-func solidImage(size int, fill color.RGBA) image.Image {
245- img := image.NewRGBA(image.Rect(0, 0, size, size))
246- for y := 0; y < size; y++ {
247- for x := 0; x < size; x++ {
248- img.SetRGBA(x, y, fill)
249- }
250- }
251- return img
252-}
253-
254-func parseHexColor(hex string) (color.RGBA, bool) {
255- v, err := strconv.ParseUint(hex, 16, 32)
256- if err != nil {
257- return color.RGBA{}, false
258- }
259- const greenShift = 8
260- return color.RGBA{R: uint8(v >> 16), G: uint8(v >> greenShift), B: uint8(v), A: 255}, true
261-}
M internal/infrastructure/favicon/converter_test.go +17 -2
@@ -39,8 +39,6 @@ func TestConverterAcceptedFaviconFormatsProducePNGs(t *testing.T) {
3939 {"png", "image/png", testPNG(t, 16, 16)},
4040 {"ico with png payload", "image/x-icon", testICOWithPNG(t, testPNG(t, 16, 16))},
4141 {"ico with bitmap payload", "image/x-icon", testICOWithDIB(t)},
42- {"svg", "image/svg+xml", []byte(`<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16"><rect width="16" height="16" fill="#ff0000"/></svg>`)},
43- {"webp placeholder", "image/webp", []byte("RIFF\x04\x00\x00\x00WEBPVP8 ")},
4442 } {
4543 t.Run(tc.name, func(t *testing.T) {
4644 out, err := conv.Convert(context.Background(), tc.data, tc.ct, []int{32})
@@ -61,6 +59,23 @@ func TestConverterAcceptedFaviconFormatsProducePNGs(t *testing.T) {
6159 }
6260 }
6361
62+func TestConverterRejectsFormatsThatWouldProducePlaceholders(t *testing.T) {
63+ conv := NewImageConverter()
64+ for _, tc := range []struct {
65+ name, ct string
66+ data []byte
67+ }{
68+ {"svg", "image/svg+xml", []byte(`<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16"><rect width="16" height="16" fill="#ff0000"/></svg>`)},
69+ {"webp", "image/webp", []byte("RIFF\x04\x00\x00\x00WEBPVP8 ")},
70+ } {
71+ t.Run(tc.name, func(t *testing.T) {
72+ if _, err := conv.Convert(context.Background(), tc.data, tc.ct, []int{32}); !errors.Is(err, appport.ErrFaviconMiss) {
73+ t.Fatalf("err = %v, want favicon miss", err)
74+ }
75+ })
76+ }
77+}
78+
6479 func TestConverterMalformedInputsAreMisses(t *testing.T) {
6580 conv := NewImageConverter()
6681 for _, tc := range []struct {
@@ -39,8 +39,6 @@ func TestConverterAcceptedFaviconFormatsProducePNGs(t *testing.T) {
39 {"png", "image/png", testPNG(t, 16, 16)},39 {"png", "image/png", testPNG(t, 16, 16)},
40 {"ico with png payload", "image/x-icon", testICOWithPNG(t, testPNG(t, 16, 16))},40 {"ico with png payload", "image/x-icon", testICOWithPNG(t, testPNG(t, 16, 16))},
41 {"ico with bitmap payload", "image/x-icon", testICOWithDIB(t)},41 {"ico with bitmap payload", "image/x-icon", testICOWithDIB(t)},
42- {"svg", "image/svg+xml", []byte(`<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16"><rect width="16" height="16" fill="#ff0000"/></svg>`)},
43- {"webp placeholder", "image/webp", []byte("RIFF\x04\x00\x00\x00WEBPVP8 ")},
44 } {42 } {
45 t.Run(tc.name, func(t *testing.T) {43 t.Run(tc.name, func(t *testing.T) {
46 out, err := conv.Convert(context.Background(), tc.data, tc.ct, []int{32})44 out, err := conv.Convert(context.Background(), tc.data, tc.ct, []int{32})
@@ -61,6 +59,23 @@ func TestConverterAcceptedFaviconFormatsProducePNGs(t *testing.T) {
61 }59 }
62 }60 }
63 61
62+func TestConverterRejectsFormatsThatWouldProducePlaceholders(t *testing.T) {
63+ conv := NewImageConverter()
64+ for _, tc := range []struct {
65+ name, ct string
66+ data []byte
67+ }{
68+ {"svg", "image/svg+xml", []byte(`<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16"><rect width="16" height="16" fill="#ff0000"/></svg>`)},
69+ {"webp", "image/webp", []byte("RIFF\x04\x00\x00\x00WEBPVP8 ")},
70+ } {
71+ t.Run(tc.name, func(t *testing.T) {
72+ if _, err := conv.Convert(context.Background(), tc.data, tc.ct, []int{32}); !errors.Is(err, appport.ErrFaviconMiss) {
73+ t.Fatalf("err = %v, want favicon miss", err)
74+ }
75+ })
76+ }
77+}
78+
64 func TestConverterMalformedInputsAreMisses(t *testing.T) {79 func TestConverterMalformedInputsAreMisses(t *testing.T) {
65 conv := NewImageConverter()80 conv := NewImageConverter()
66 for _, tc := range []struct {81 for _, tc := range []struct {

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 8

Tags 42

↑↓ moveenter openesc close