From 44fcbfc4f647919d98a1325d1f2209643df3443d Mon Sep 17 00:00:00 2001 From: Julien Herr Date: Mon, 25 May 2026 23:49:43 +0200 Subject: [PATCH] fix(favicon): fall back to apex domain when subdomain hosts no icon Senders on a subdomain that hosts no favicon (e.g. mail.example.com) left feeds blank because both the direct /favicon.ico and the DuckDuckGo lookup were tried only against the full subdomain. Resolution now walks up to the apex via Domain.parents() and caches the result under the original sender domain. Co-Authored-By: Claude Opus 4.7 --- CHANGELOG.md | 5 +++ src/domain/value-objects/domain.test.ts | 39 ++++++++++++++++++++++ src/domain/value-objects/domain.ts | 16 +++++++++ src/infrastructure/favicon-fetcher.test.ts | 22 ++++++++++++ src/infrastructure/favicon-fetcher.ts | 28 ++++++++++------ 5 files changed, 100 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 61ffd7d..2f68e6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,11 @@ verbatim as the GitHub Release notes — so what you write here is what ships. ### Fixed +- Per-feed favicons now resolve for senders on a subdomain that hosts no icon of + its own (e.g. `mail.example.com`): the lookup walks up to the apex domain + (`example.com`) and uses its favicon, caching it under the original sender + domain. Previously both the direct `/favicon.ico` and the DuckDuckGo lookup + were tried only against the full subdomain, leaving such feeds blank. - Subscription-confirmation detection now flags code-based signup verifications (OTP) that have no link to click — e.g. "Your verification code is 371404", whose only link is a `mailto:` support address. These cleared the keyword diff --git a/src/domain/value-objects/domain.test.ts b/src/domain/value-objects/domain.test.ts index 894c04e..ca09883 100644 --- a/src/domain/value-objects/domain.test.ts +++ b/src/domain/value-objects/domain.test.ts @@ -22,4 +22,43 @@ describe("Domain", () => { ).toBe(true); expect(Domain.parse("a.com")!.matches(Domain.parse("b.com")!)).toBe(false); }); + + describe("parents", () => { + it("yields the domain itself and each parent, most-specific first", () => { + expect( + Domain.parse("mail.example.com")! + .parents() + .map((d) => d.value), + ).toEqual(["mail.example.com", "example.com"]); + }); + + it("stops at the two-label registrable domain", () => { + expect( + Domain.parse("a.b.c.example.com")! + .parents() + .map((d) => d.value), + ).toEqual([ + "a.b.c.example.com", + "b.c.example.com", + "c.example.com", + "example.com", + ]); + }); + + it("returns just the domain when it is already two labels", () => { + expect( + Domain.parse("example.com")! + .parents() + .map((d) => d.value), + ).toEqual(["example.com"]); + }); + + it("returns the single label as-is", () => { + expect( + Domain.parse("localhost")! + .parents() + .map((d) => d.value), + ).toEqual(["localhost"]); + }); + }); }); diff --git a/src/domain/value-objects/domain.ts b/src/domain/value-objects/domain.ts index fe278a4..4ec3e03 100644 --- a/src/domain/value-objects/domain.ts +++ b/src/domain/value-objects/domain.ts @@ -18,6 +18,22 @@ export class Domain { return this.value === other.value; } + /** + * This domain plus each parent domain down to the two-label registrable + * level, most-specific first: `a.b.example.com` → + * `[a.b.example.com, b.example.com, example.com]`. Lets a lookup fall back to + * the apex when a sending subdomain (e.g. `mail.example.com`) hosts no asset + * of its own. A single-label value is returned unchanged. + */ + parents(): Domain[] { + const labels = this.value.split("."); + const result: Domain[] = []; + for (let i = 0; i + 2 <= labels.length; i++) { + result.push(new Domain(labels.slice(i).join("."))); + } + return result.length ? result : [this]; + } + toString(): string { return this.value; } diff --git a/src/infrastructure/favicon-fetcher.test.ts b/src/infrastructure/favicon-fetcher.test.ts index 408a6bd..b0375b7 100644 --- a/src/infrastructure/favicon-fetcher.test.ts +++ b/src/infrastructure/favicon-fetcher.test.ts @@ -76,6 +76,28 @@ describe("cacheFaviconForDomain", () => { expect(icon?.contentType).toBe("image/x-icon"); }); + it("falls back to the apex domain when the subdomain has no icon", async () => { + const env = createMockEnv() as unknown as Env; + server.use( + http.get("https://mail.acme.test/favicon.ico", () => + HttpResponse.error(), + ), + http.get("https://icons.duckduckgo.com/ip3/mail.acme.test.ico", () => + HttpResponse.text("", { status: 404 }), + ), + http.get("https://acme.test/favicon.ico", () => + imageResponse(PNG, "image/vnd.microsoft.icon"), + ), + ); + + await cacheFaviconForDomain("mail.acme.test", env); + + // Cached under the original sender domain, so reads still hit. + const icon = await getCachedIcon("mail.acme.test", env); + expect(icon?.contentType).toBe("image/vnd.microsoft.icon"); + expect(new Uint8Array(icon!.bytes)).toEqual(PNG); + }); + it("writes a negative entry when no icon is found", async () => { const env = createMockEnv() as unknown as Env; server.use( diff --git a/src/infrastructure/favicon-fetcher.ts b/src/infrastructure/favicon-fetcher.ts index 60a1c5a..d4b258d 100644 --- a/src/infrastructure/favicon-fetcher.ts +++ b/src/infrastructure/favicon-fetcher.ts @@ -6,6 +6,7 @@ import { MAX_ICON_BYTES, } from "../config/constants"; import { IconRepository } from "./icon-repository"; +import { Domain } from "../domain/value-objects/domain"; import { EmailAddress } from "../domain/value-objects/email-address"; import { logger } from "./logger"; @@ -65,16 +66,23 @@ async function fetchIconFrom( async function resolveIcon( domain: string, ): Promise<{ buffer: ArrayBuffer; contentType: string } | null> { - const candidates = [ - `https://${domain}/favicon.ico`, - `https://icons.duckduckgo.com/ip3/${domain}.ico`, - ]; - for (const url of candidates) { - try { - const icon = await fetchIconFrom(url); - if (icon) return icon; - } catch { - // Try the next candidate; network/timeout errors must never propagate. + // Walk the sending subdomain up to its apex so a sender like + // `mail.example.com` falls back to `example.com`'s favicon. + const hosts = Domain.parse(domain) + ?.parents() + .map((d) => d.value) ?? [domain]; + for (const host of hosts) { + const candidates = [ + `https://${host}/favicon.ico`, + `https://icons.duckduckgo.com/ip3/${host}.ico`, + ]; + for (const url of candidates) { + try { + const icon = await fetchIconFrom(url); + if (icon) return icon; + } catch { + // Try the next candidate; network/timeout errors must never propagate. + } } } return null;