mirror of
https://github.com/juherr/kill-the-news.git
synced 2026-06-20 22:03:48 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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"]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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,9 +66,15 @@ async function fetchIconFrom(
|
||||
async function resolveIcon(
|
||||
domain: string,
|
||||
): Promise<{ buffer: ArrayBuffer; contentType: string } | null> {
|
||||
// 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://${domain}/favicon.ico`,
|
||||
`https://icons.duckduckgo.com/ip3/${domain}.ico`,
|
||||
`https://${host}/favicon.ico`,
|
||||
`https://icons.duckduckgo.com/ip3/${host}.ico`,
|
||||
];
|
||||
for (const url of candidates) {
|
||||
try {
|
||||
@@ -77,6 +84,7 @@ async function resolveIcon(
|
||||
// Try the next candidate; network/timeout errors must never propagate.
|
||||
}
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user