diff --git a/src/node/hooks/express/specialpages.js b/src/node/hooks/express/specialpages.js index b49132e91..725139c09 100644 --- a/src/node/hooks/express/specialpages.js +++ b/src/node/hooks/express/specialpages.js @@ -2,9 +2,12 @@ const path = require('path'); const eejs = require('../../eejs'); +const fs = require('fs'); +const fsp = fs.promises; const toolbar = require('../../utils/toolbar'); const hooks = require('../../../static/js/pluginfw/hooks'); const settings = require('../../utils/Settings'); +const util = require('util'); const webaccess = require('./webaccess'); exports.expressCreateServer = (hookName, args, cb) => { @@ -74,23 +77,24 @@ exports.expressCreateServer = (hookName, args, cb) => { })); }); - args.app.get('/favicon.ico', (req, res) => { - let filePath = path.join( - settings.root, - 'src', - 'static', - 'skins', - settings.skinName, - 'favicon.ico' - ); - res.setHeader('Cache-Control', `public, max-age=${settings.maxAge}`); - res.sendFile(filePath, (err) => { - // there is no custom favicon, send the default favicon - if (err) { - filePath = path.join(settings.root, 'src', 'static', 'favicon.ico'); - res.sendFile(filePath); + args.app.get('/favicon.ico', (req, res, next) => { + (async () => { + const fns = [ + path.join(settings.root, 'src', 'static', 'skins', settings.skinName, 'favicon.ico'), + path.join(settings.root, 'src', 'static', 'favicon.ico'), + ]; + for (const fn of fns) { + try { + await fsp.access(fn, fs.constants.R_OK); + } catch (err) { + continue; + } + res.setHeader('Cache-Control', `public, max-age=${settings.maxAge}`); + await util.promisify(res.sendFile.bind(res))(fn); + return; } - }); + next(); + })().catch((err) => next(err || new Error(err))); }); return cb(); diff --git a/src/tests/backend/specs/favicon-test-skin.png b/src/tests/backend/specs/favicon-test-skin.png new file mode 100644 index 000000000..87bdadbbb Binary files /dev/null and b/src/tests/backend/specs/favicon-test-skin.png differ diff --git a/src/tests/backend/specs/favicon.js b/src/tests/backend/specs/favicon.js new file mode 100644 index 000000000..76140b72f --- /dev/null +++ b/src/tests/backend/specs/favicon.js @@ -0,0 +1,55 @@ +'use strict'; + +const assert = require('assert').strict; +const common = require('../common'); +const fs = require('fs'); +const fsp = fs.promises; +const path = require('path'); +const settings = require('../../../node/utils/Settings'); +const superagent = require('superagent'); + +describe(__filename, function () { + let agent; + let backupSettings; + let skinDir; + let wantDefaultIcon; + let wantSkinIcon; + + before(async function () { + agent = await common.init(); + wantDefaultIcon = await fsp.readFile(path.join(settings.root, 'src', 'static', 'favicon.ico')); + wantSkinIcon = await fsp.readFile(path.join(__dirname, 'favicon-test-skin.png')); + }); + + beforeEach(async function () { + backupSettings = {...settings}; + skinDir = await fsp.mkdtemp(path.join(settings.root, 'src', 'static', 'skins', 'test-')); + settings.skinName = path.basename(skinDir); + }); + + afterEach(async function () { + delete settings.skinName; + Object.assign(settings, backupSettings); + try { + // TODO: The {recursive: true} option wasn't added to fsp.rmdir() until Node.js v12.10.0 so we + // can't rely on it until support for Node.js v10 is dropped. + await fsp.unlink(path.join(skinDir, 'favicon.ico')); + await fsp.rmdir(skinDir, {recursive: true}); + } catch (err) { /* intentionally ignored */ } + }); + + it('uses skin favicon if present', async function () { + await fsp.writeFile(path.join(skinDir, 'favicon.ico'), wantSkinIcon); + const {body: gotIcon} = await agent.get('/favicon.ico') + .accept('png').buffer(true).parse(superagent.parse.image) + .expect(200); + assert(gotIcon.equals(wantSkinIcon)); + }); + + it('falls back to default favicon', async function () { + const {body: gotIcon} = await agent.get('/favicon.ico') + .accept('png').buffer(true).parse(superagent.parse.image) + .expect(200); + assert(gotIcon.equals(wantDefaultIcon)); + }); +});