Skip to content

Commit 79a7aa1

Browse files
authored
improvement(favicons): resolve platform tenants no provider can see, retry timeout-derived icons, no miss flash (#9497)
* improvement(favicons): resolve platform tenants no provider can see, and retry icons chosen after a timeout A tenant on a platform whose domain has no icon at either provider, even at the parent (AWS sign-in portals, Coupa, GitLab Dedicated, Databricks apps, Salesforce, Okta preview, UKG, Jama), and a self-hosted gitlab./jira./testrail. host, now end their candidate walk at the platform's own site, after their own host and parents so a tenant's custom icon still wins. A parent's icon reached only because a more specific lookup timed out is cached for a day instead of thirty, since the host's own icon may exist. * improvement(favicons): correct the platform table from vendor docs and keep misses from flashing GitLab Dedicated instances live on gitlab-dedicated.com (and .systems pre-production); .site is their Pages domain, kept to match gitlab.io. Adds coupacloud.com, databricks.us, databricksapps.us, and testrail.io, each a suffix both providers miss at every level; drops entries the parent walk already resolves and the self-hosted sentry label, whose name collides with other products. A host with no icon is remembered client-side for the route's miss lifetime, so a remounted icon shows its fallback without an empty frame, and the cache key moves to v3 so stale misses for newly mapped platforms clear on deploy. * fix(favicons): keep provisional icons provisional through reuse, walk past cached parent misses, skip hosting suffixes A cached hit now records whether it was definite or provisional, so a host reusing a parent's timeout-derived icon inherits the one-day lifetime instead of a month. A cached miss at a parent skips only the candidates that parent's own walk covered, so a product fallback derived from the original host is still tried. The self-hosted product label no longer applies on a Public Suffix List private (hosting) suffix. Adds the Databricks military suffix the repo already recognizes, and the client remembers a failed icon only as long as the route retries an outage. * chore(favicons): import resolver test mocks from their own files
1 parent 26ba3a9 commit 79a7aa1

7 files changed

Lines changed: 266 additions & 18 deletions

File tree

‎apps/sim/components/favicon.tsx‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,16 @@
22

33
import { type ReactNode, useState } from 'react'
44
import { cn } from '@sim/emcn'
5+
import { LRUCache } from 'lru-cache'
56
import { faviconSrc } from '@/lib/favicon/url'
67

8+
/**
9+
* Hosts whose icon just failed, so an icon that mounts again renders its fallback at once
10+
* instead of an empty slot first. An `<img>` can't tell no icon from a provider outage, so this
11+
* lasts only as long as the route retries an outage; a confirmed miss stays in the HTTP cache.
12+
*/
13+
const knownMisses = new LRUCache<string, true>({ max: 500, ttl: 5 * 60 * 1000 })
14+
715
interface FaviconProps {
816
/** The site's hostname; without one the fallback renders. */
917
host: string | null | undefined
@@ -20,15 +28,18 @@ interface FaviconProps {
2028
*/
2129
export function Favicon({ host, fallback, className }: FaviconProps) {
2230
const [failedHost, setFailedHost] = useState<string | null>(null)
23-
if (!host || failedHost === host) return fallback
31+
if (!host || failedHost === host || knownMisses.has(host)) return fallback
2432
return (
2533
<img
2634
src={faviconSrc(host)}
2735
alt=''
2836
loading='lazy'
2937
decoding='async'
3038
className={cn('rounded-sm', className)}
31-
onError={() => setFailedHost(host)}
39+
onError={() => {
40+
knownMisses.set(host, true)
41+
setFailedHost(host)
42+
}}
3243
/>
3344
)
3445
}

‎apps/sim/lib/favicon/hosts.test.ts‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,79 @@ describe('faviconHostCandidates', () => {
4040
])
4141
})
4242

43+
it.each([
44+
['portal.awsapps.com', ['portal.awsapps.com', 'awsapps.com', 'aws.amazon.com']],
45+
['acme.coupahost.com', ['acme.coupahost.com', 'coupahost.com', 'coupa.com']],
46+
[
47+
'docs.acme.gitlab-dedicated.site',
48+
[
49+
'docs.acme.gitlab-dedicated.site',
50+
'acme.gitlab-dedicated.site',
51+
'gitlab-dedicated.site',
52+
'gitlab.com',
53+
],
54+
],
55+
[
56+
'acme.gitlab-dedicated.com',
57+
['acme.gitlab-dedicated.com', 'gitlab-dedicated.com', 'gitlab.com'],
58+
],
59+
['acme.testrail.io', ['acme.testrail.io', 'testrail.io', 'testrail.com']],
60+
[
61+
'acme.databricksapps.mil',
62+
['acme.databricksapps.mil', 'databricksapps.mil', 'databricks.com'],
63+
],
64+
[
65+
'dbc-1.cloud.databricks.us',
66+
['dbc-1.cloud.databricks.us', 'cloud.databricks.us', 'databricks.us', 'databricks.com'],
67+
],
68+
])(
69+
'ends a platform tenant %s at the platform, after its own host and parents',
70+
(host, expected) => {
71+
expect(faviconHostCandidates(host)).toEqual(expected)
72+
}
73+
)
74+
75+
it.each([
76+
['gitlab.acme-internal.io', 'gitlab.com'],
77+
['jira.acme-internal.io', 'atlassian.com'],
78+
['testrail.qa.acme-internal.io', 'testrail.com'],
79+
])('ends a self-hosted %s at its product', (host, product) => {
80+
expect(faviconHostCandidates(host).at(-1)).toBe(product)
81+
})
82+
83+
it.each([
84+
'mygitlab.acme-internal.io',
85+
'docs.gitlab.acme-internal.io',
86+
'notawsapps.com',
87+
'acme.herokuapp.com',
88+
'gitlab.azurewebsites.net',
89+
'gitlab.acme.vercel.app',
90+
])('adds no platform for %s, which only resembles one', (host) => {
91+
const candidates = faviconHostCandidates(host)
92+
expect(candidates).not.toContain('gitlab.com')
93+
expect(candidates).not.toContain('aws.amazon.com')
94+
expect(candidates.length).toBeGreaterThan(0)
95+
})
96+
97+
it.each(['gitlab.corp', 'gitlab', 'jira.local'])(
98+
'still offers nothing for internal %s',
99+
(host) => {
100+
expect(faviconHostCandidates(host)).toEqual([])
101+
}
102+
)
103+
104+
it('adds nothing for a platform whose parent the walk already reaches', () => {
105+
expect(faviconHostCandidates('acme.my.salesforce.com')).toEqual([
106+
'acme.my.salesforce.com',
107+
'my.salesforce.com',
108+
'salesforce.com',
109+
])
110+
})
111+
112+
it('never repeats a host the walk already reaches', () => {
113+
expect(faviconHostCandidates('gitlab.gitlab.com')).toEqual(['gitlab.gitlab.com', 'gitlab.com'])
114+
})
115+
43116
it('tries at most three parents', () => {
44117
expect(faviconHostCandidates('a.b.c.d.e.example.com')).toEqual([
45118
'a.b.c.d.e.example.com',

‎apps/sim/lib/favicon/hosts.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
import { parse } from 'tldts'
2+
import { platformSite } from '@/lib/favicon/platforms'
23

34
const MAX_PARENT_HOSTS = 3
45
const HOST_SHAPE = /^[^\s/?#@\\]+$/
56

67
/**
78
* Hosts to ask a favicon provider about, most specific first. A private tenant such as
89
* `acme.atlassian.net` is invisible to providers, but its platform is not, so each parent
9-
* domain follows — at most three, stopping before the ICANN public suffix (`gitlab.io` is a valid last step;
10-
* `io` and `co.uk` are not). Internal hosts — IP literals, single labels, and names outside
10+
* domain follows — at most three, stopping before the ICANN public suffix (`gitlab.io` is a valid
11+
* last step; `io` and `co.uk` are not). A host on a platform no provider can see even at the
12+
* parent (`x.awsapps.com`, a self-hosted `gitlab.<company>`) ends at the platform's own site, after
13+
* its own host and parents so a tenant's custom icon still wins. Internal hosts — IP literals, single labels, and names outside
1114
* the public DNS — yield nothing, so they never leave the server, and neither does a name with
1215
* characters DNS can't hold, which could otherwise reshape a provider's URL.
1316
*/
@@ -23,7 +26,9 @@ export function faviconHostCandidates(input: string): string[] {
2326
if (isIp || !isIcann || !publicSuffix || /(^|\.)arpa$/.test(publicSuffix)) return []
2427
const labels = hostname.split('.')
2528
const candidateCount = labels.length - publicSuffix.split('.').length
26-
return labels
29+
const candidates = labels
2730
.slice(0, Math.min(candidateCount, MAX_PARENT_HOSTS + 1))
2831
.map((_, index) => labels.slice(index).join('.'))
32+
const platform = platformSite(hostname)
33+
return platform && !candidates.includes(platform) ? [...candidates, platform] : candidates
2934
}

‎apps/sim/lib/favicon/platforms.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { parse } from 'tldts'
2+
3+
/**
4+
* Platforms whose tenant hosts no icon provider can see — not even at any parent the walk
5+
* reaches — mapped to the platform's own site, so a tenant still shows the product it is. A
6+
* platform whose parent already resolves (`salesforce.com`, `databricks.com`, `atlassian.net`)
7+
* needs no entry. Generic hosting domains (`herokuapp.com`, `azurewebsites.net`,
8+
* `cloudfront.net`) are deliberately absent: they serve other companies' apps, and the host's
9+
* logo would misrepresent them. GitLab Pages on Dedicated (`gitlab-dedicated.site`) maps like
10+
* `gitlab.io`, whose parent already shows GitLab's icon.
11+
*/
12+
const PLATFORM_SITES: ReadonlyMap<string, string> = new Map([
13+
['awsapps.com', 'aws.amazon.com'],
14+
['azuredatabricks.net', 'databricks.com'],
15+
['coupacloud.com', 'coupa.com'],
16+
['coupahost.com', 'coupa.com'],
17+
['databricks.us', 'databricks.com'],
18+
['databricksapps.com', 'databricks.com'],
19+
['databricksapps.mil', 'databricks.com'],
20+
['databricksapps.us', 'databricks.com'],
21+
['gitlab-dedicated.com', 'gitlab.com'],
22+
['gitlab-dedicated.site', 'gitlab.com'],
23+
['gitlab-dedicated.systems', 'gitlab.com'],
24+
['jamacloud.com', 'jamasoftware.com'],
25+
['oktapreview.com', 'okta.com'],
26+
['testrail.io', 'testrail.com'],
27+
['ultipro.com', 'ukg.com'],
28+
])
29+
30+
/**
31+
* Self-hosted tools conventionally served at `<product>.<company domain>` — only on a company's
32+
* own domain, never on a hosting platform's suffix (`gitlab.azurewebsites.net` is someone's app).
33+
*/
34+
const SELF_HOSTED_PRODUCTS: ReadonlyMap<string, string> = new Map([
35+
['confluence', 'atlassian.com'],
36+
['gitlab', 'gitlab.com'],
37+
['grafana', 'grafana.com'],
38+
['jenkins', 'jenkins.io'],
39+
['jira', 'atlassian.com'],
40+
['testrail', 'testrail.com'],
41+
])
42+
43+
/** The public site of the platform a host belongs to, matched on whole labels, or `null`. */
44+
export function platformSite(hostname: string): string | null {
45+
const labels = hostname.split('.')
46+
for (let index = 0; index < labels.length; index++) {
47+
const site = PLATFORM_SITES.get(labels.slice(index).join('.'))
48+
if (site) return site
49+
}
50+
if (parse(hostname, { allowPrivateDomains: true }).isPrivate) return null
51+
return SELF_HOSTED_PRODUCTS.get(labels[0]) ?? null
52+
}
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import {
2+
inputValidationMock,
3+
inputValidationMockFns,
4+
} from '@sim/testing/mocks/input-validation.mock'
5+
import { createMockRedis } from '@sim/testing/mocks/redis.mock'
6+
import { redisConfigMockFns } from '@sim/testing/mocks/redis-config.mock'
7+
import { beforeEach, describe, expect, it, vi } from 'vitest'
8+
import { resolveFavicon } from '@/lib/favicon/resolve.server'
9+
10+
vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
11+
12+
const PNG = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0, 0, 0, 0])
13+
const DAY = 24 * 60 * 60
14+
15+
type Answer = 'icon' | 'none' | 'timeout'
16+
17+
/** Each host's answer at both providers; any host not listed has no icon. */
18+
let answers: Record<string, Answer> = {}
19+
let stored: Map<string, { value: Buffer; ttl: number }>
20+
21+
function hostOf(url: string): string {
22+
const match = url.match(/url=https:\/\/([^&]+)&/) ?? url.match(/\/ip3\/(.+)\.ico$/)
23+
if (!match) throw new Error(`Unexpected provider URL ${url}`)
24+
return match[1]
25+
}
26+
27+
beforeEach(() => {
28+
answers = {}
29+
stored = new Map()
30+
const redis = createMockRedis()
31+
redis.getBuffer.mockImplementation(async (key: string) => stored.get(key)?.value ?? null)
32+
redis.set.mockImplementation(
33+
async (key: string, value: Buffer | string, _ex: string, ttl: number) => {
34+
stored.set(key, { value: Buffer.from(value), ttl })
35+
return 'OK'
36+
}
37+
)
38+
redisConfigMockFns.mockGetRedisClient.mockReturnValue(redis)
39+
inputValidationMockFns.mockSecureFetchWithValidation.mockImplementation(async (url: string) => {
40+
const answer = answers[hostOf(url)] ?? 'none'
41+
if (answer === 'timeout') throw new Error('timed out')
42+
return {
43+
status: answer === 'icon' ? 200 : 404,
44+
arrayBuffer: async () => PNG.buffer.slice(PNG.byteOffset, PNG.byteOffset + PNG.byteLength),
45+
body: { cancel: async () => {} },
46+
}
47+
})
48+
})
49+
50+
const ttlOf = (host: string) => stored.get(`favicon:v3:${host}`)?.ttl
51+
52+
describe('resolveFavicon', () => {
53+
it('keeps a parent icon chosen after a timeout provisional when a child host reuses it', async () => {
54+
answers = { 'gitlab.acme-internal.io': 'timeout', 'gitlab.com': 'icon' }
55+
expect((await resolveFavicon('gitlab.acme-internal.io')).kind).toBe('hit')
56+
expect(ttlOf('gitlab.acme-internal.io')).toBe(DAY)
57+
58+
expect((await resolveFavicon('docs.gitlab.acme-internal.io')).kind).toBe('hit')
59+
expect(ttlOf('docs.gitlab.acme-internal.io')).toBe(DAY)
60+
})
61+
62+
it('still reaches the product fallback after its parent was cached as a miss', async () => {
63+
answers = { 'atlassian.com': 'icon' }
64+
expect((await resolveFavicon('acme-internal.io')).kind).toBe('miss')
65+
66+
expect((await resolveFavicon('jira.acme-internal.io')).kind).toBe('hit')
67+
})
68+
69+
it('remembers a definite icon for a month', async () => {
70+
answers = { 'acme.example.com': 'icon' }
71+
expect((await resolveFavicon('acme.example.com')).kind).toBe('hit')
72+
expect(ttlOf('acme.example.com')).toBe(30 * DAY)
73+
})
74+
})

‎apps/sim/lib/favicon/resolve.server.ts‎

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,15 @@ const PROVIDER_TIMEOUT_MS = 1500
1212
const RESOLVE_DEADLINE_MS = 4000
1313
const HIT_TTL_SECONDS = 30 * 24 * 60 * 60
1414
const MISS_TTL_SECONDS = 24 * 60 * 60
15+
/** A parent's icon chosen because the host's own lookup timed out: retried a day later, not a month. */
16+
const PROVISIONAL_HIT_TTL_SECONDS = 24 * 60 * 60
1517
const UNAVAILABLE_TTL_SECONDS = 5 * 60
16-
/** Cached values: an icon's bytes, or one of these markers. */
18+
/**
19+
* Cached values: a hit is one prefix byte — definite or provisional, which a host reusing it
20+
* must inherit — followed by the icon; the markers stand for no icon and for an outage.
21+
*/
22+
const DEFINITE_HIT_PREFIX = 'h'
23+
const PROVISIONAL_HIT_PREFIX = 'p'
1724
const MISS_MARKER = ''
1825
const UNAVAILABLE_MARKER = '-'
1926

@@ -28,13 +35,13 @@ const PROVIDERS: readonly ((host: string) => string)[] = [
2835
]
2936

3037
type FaviconResolution =
31-
| { kind: 'hit'; contentType: string; bytes: Buffer }
38+
| { kind: 'hit'; contentType: string; bytes: Buffer; provisional?: true }
3239
| { kind: 'miss' }
3340
| { kind: 'unavailable' }
3441

3542
const inFlight = new Map<string, Promise<FaviconResolution>>()
3643

37-
const cacheKey = (host: string) => `favicon:v2:${host}`
44+
const cacheKey = (host: string) => `favicon:v3:${host}`
3845

3946
/** A host's key holds the result of the walk that starts at that host. */
4047
async function readCache(host: string): Promise<FaviconResolution | null> {
@@ -44,8 +51,13 @@ async function readCache(host: string): Promise<FaviconResolution | null> {
4451
const cached = await redis.getBuffer(cacheKey(host))
4552
if (cached === null) return null
4653
if (cached.toString() === UNAVAILABLE_MARKER) return { kind: 'unavailable' }
47-
const result = classifyProviderResponse(200, cached)
48-
return result.kind === 'hit' ? { ...result, bytes: cached } : { kind: 'miss' }
54+
const prefix = cached.subarray(0, 1).toString()
55+
const bytes = cached.subarray(1)
56+
const result = classifyProviderResponse(200, bytes)
57+
if (result.kind !== 'hit') return { kind: 'miss' }
58+
return prefix === PROVISIONAL_HIT_PREFIX
59+
? { ...result, bytes, provisional: true }
60+
: { ...result, bytes }
4961
} catch (error) {
5062
logger.warn('Favicon cache read failed', { error })
5163
return null
@@ -57,7 +69,15 @@ async function writeCache(host: string, resolution: FaviconResolution): Promise<
5769
if (!redis) return
5870
const [value, ttl] =
5971
resolution.kind === 'hit'
60-
? ([resolution.bytes, HIT_TTL_SECONDS] as const)
72+
? resolution.provisional
73+
? ([
74+
Buffer.concat([Buffer.from(PROVISIONAL_HIT_PREFIX), resolution.bytes]),
75+
PROVISIONAL_HIT_TTL_SECONDS,
76+
] as const)
77+
: ([
78+
Buffer.concat([Buffer.from(DEFINITE_HIT_PREFIX), resolution.bytes]),
79+
HIT_TTL_SECONDS,
80+
] as const)
6181
: resolution.kind === 'miss'
6282
? ([MISS_MARKER, MISS_TTL_SECONDS] as const)
6383
: ([UNAVAILABLE_MARKER, UNAVAILABLE_TTL_SECONDS] as const)
@@ -101,29 +121,41 @@ async function askProviders(host: string, signal: AbortSignal): Promise<FaviconR
101121

102122
/**
103123
* The most specific host first, so a tenant's own icon beats its platform's. A parent already
104-
* resolved for a sibling subdomain is reused from the cache instead of asked again.
124+
* resolved for a sibling subdomain is reused from the cache instead of asked again. A parent's
125+
* icon reached only because a more specific lookup was unavailable is `provisional`: the host's
126+
* own icon may exist, so it is not remembered for long.
105127
*/
106128
async function walk(candidates: string[], signal: AbortSignal): Promise<FaviconResolution> {
107129
let unavailable = false
130+
const covered = new Set<string>()
108131
for (const [index, host] of candidates.entries()) {
132+
if (covered.has(host)) continue
109133
const parent = index > 0
110134
const cached = parent ? await readCache(host) : null
111-
if (cached) return cached.kind === 'miss' && unavailable ? { kind: 'unavailable' } : cached
135+
if (cached?.kind === 'hit') {
136+
return unavailable || cached.provisional ? { ...cached, provisional: true } : cached
137+
}
138+
if (cached) {
139+
if (cached.kind === 'unavailable') unavailable = true
140+
for (const reached of faviconHostCandidates(host)) covered.add(reached)
141+
continue
142+
}
112143
const result = await askProviders(host, signal)
113144
if (result.kind === 'hit') {
114145
if (parent) await writeCache(host, result)
115-
return result
146+
return unavailable ? { ...result, provisional: true } : result
116147
}
117148
if (result.kind === 'unavailable') unavailable = true
118149
}
119150
return unavailable ? { kind: 'unavailable' } : { kind: 'miss' }
120151
}
121152

122153
/**
123-
* The icon for a host, or its nearest parent domain's, from public icon providers — the
124-
* destination itself is never contacted, so a host behind a login still gets its platform's
125-
* icon. Results are cached per host (30 days for an icon, a day for none, minutes after an
126-
* outage), and concurrent lookups of one host in this process share a single resolution.
154+
* The icon for a host, or its nearest parent domain's, from public icon providers — Sim never
155+
* contacts the destination itself (a provider may, from its own servers), so a host behind a
156+
* login still gets its platform's icon. Results are cached per host (30 days for an icon, a day
157+
* for none or for a parent's icon chosen after a timeout, minutes after an outage), and
158+
* concurrent lookups of one host in this process share a single resolution.
127159
*/
128160
export async function resolveFavicon(host: string): Promise<FaviconResolution> {
129161
const candidates = faviconHostCandidates(host)

‎packages/testing/src/mocks/redis.mock.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ export function createMockRedis() {
3030

3131
// Key operations
3232
get: vi.fn().mockResolvedValue(null),
33+
getBuffer: vi.fn().mockResolvedValue(null),
3334
set: vi.fn().mockResolvedValue('OK'),
3435
del: vi.fn().mockResolvedValue(1),
3536
exists: vi.fn().mockResolvedValue(0),

0 commit comments

Comments
 (0)