fix(branding): stream favicon bytes directly (Safari ignores the 302)
The /favicon.ico route 302-redirected to the uploaded file. Firefox/Chrome follow that, but Safari does NOT reliably follow a redirect for favicon requests — it falls back to the HTML <link>, i.e. the bundled picpeak default. Stream the file bytes directly for local /uploads favicons (with a path-containment guard); only external URLs and the missing-favicon fallback still redirect. sendFile sets the content-type from the extension.
This commit is contained in:
+15
-6
@@ -573,12 +573,21 @@ app.get(
|
|||||||
const raw = await getAppSetting('branding_favicon_url', null);
|
const raw = await getAppSetting('branding_favicon_url', null);
|
||||||
const url = (raw && String(raw).trim()) || null;
|
const url = (raw && String(raw).trim()) || null;
|
||||||
if (url) {
|
if (url) {
|
||||||
// Absolute URL → redirect as-is. Otherwise it's an /uploads path the
|
// External URL — can't stream the bytes, so redirect (best effort).
|
||||||
// backend already serves with the correct content-type + headers.
|
if (/^https?:\/\//i.test(url)) return res.redirect(302, url);
|
||||||
const target = /^https?:\/\//i.test(url)
|
// Local upload → stream the file bytes DIRECTLY rather than 302'ing.
|
||||||
? url
|
// Safari does NOT reliably follow a redirect for favicon requests
|
||||||
: (url.startsWith('/') ? url : `/uploads/${url.replace(/^uploads\//, '')}`);
|
// (it falls back to the HTML <link>, i.e. the bundled default),
|
||||||
return res.redirect(302, target);
|
// whereas Firefox/Chrome do — so a 302 worked everywhere except
|
||||||
|
// Safari. sendFile sets the right content-type from the extension.
|
||||||
|
const rel = String(url).replace(/^\/+/, '').replace(/^uploads\//, '');
|
||||||
|
const uploadsRoot = path.resolve(path.join(storagePath, 'uploads'));
|
||||||
|
const resolved = path.resolve(path.join(uploadsRoot, rel));
|
||||||
|
// Path containment — never serve outside the uploads dir.
|
||||||
|
if (resolved.startsWith(uploadsRoot + path.sep) && fs.existsSync(resolved)) {
|
||||||
|
res.setHeader('Cache-Control', 'public, max-age=86400');
|
||||||
|
return res.sendFile(resolved);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
logger.warn('Favicon lookup failed; serving bundled default', { error: error.message });
|
logger.warn('Favicon lookup failed; serving bundled default', { error: error.message });
|
||||||
|
|||||||
Reference in New Issue
Block a user