diff --git a/src/controllers/write/topics.js b/src/controllers/write/topics.js index 974e6d1734..fbf21e333b 100644 --- a/src/controllers/write/topics.js +++ b/src/controllers/write/topics.js @@ -7,6 +7,7 @@ const topics = require('../../topics'); const privileges = require('../../privileges'); const helpers = require('../helpers'); +const middleware = require('../../middleware/assert'); const uploadsController = require('../uploads'); const Topics = module.exports; @@ -105,7 +106,11 @@ Topics.addThumb = async (req, res) => { // Add uploaded files to topic zset if (files && files.length) { await Promise.all(files.map(async (fileObj) => { - await topics.thumbs.associate(req.params.tid, fileObj.path); + await topics.thumbs.associate({ + id: req.params.tid, + path: fileObj.path || null, + url: fileObj.url, + }); })); } }; @@ -124,6 +129,13 @@ Topics.migrateThumbs = async (req, res) => { }; Topics.deleteThumb = async (req, res) => { + if (!req.body.path.startsWith('http')) { + await middleware.assert.path(req, res); + if (res.headersSent) { + return; + } + } + await checkThumbPrivileges({ tid: req.params.tid, uid: req.user.uid, res }); if (res.headersSent) { return; diff --git a/src/routes/write/topics.js b/src/routes/write/topics.js index 4ea061e911..9203146588 100644 --- a/src/routes/write/topics.js +++ b/src/routes/write/topics.js @@ -37,7 +37,7 @@ module.exports = function () { setupApiRoute(router, 'get', '/:tid/thumbs', [], controllers.write.topics.getThumbs); setupApiRoute(router, 'post', '/:tid/thumbs', [multipartMiddleware, middleware.validateFiles, ...middlewares], controllers.write.topics.addThumb); setupApiRoute(router, 'put', '/:tid/thumbs', [], controllers.write.topics.migrateThumbs); - setupApiRoute(router, 'delete', '/:tid/thumbs', [...middlewares, middleware.assert.path], controllers.write.topics.deleteThumb); + setupApiRoute(router, 'delete', '/:tid/thumbs', [...middlewares, middleware.checkRequired.bind(null, ['path'])], controllers.write.topics.deleteThumb); return router; }; diff --git a/src/topics/thumbs.js b/src/topics/thumbs.js index 4b56f454af..c3ddd5a0b4 100644 --- a/src/topics/thumbs.js +++ b/src/topics/thumbs.js @@ -32,23 +32,29 @@ Thumbs.get = async function (tids) { let response = thumbs.map((thumbSet, idx) => thumbSet.map(thumb => ({ id: tids[idx], name: path.basename(thumb), - url: path.join(nconf.get('upload_url'), thumb), + url: thumb.startsWith('http') ? thumb : path.join(nconf.get('upload_url'), thumb), }))); ({ thumbs: response } = await plugins.hooks.fire('filter:topics.getThumbs', { tids, thumbs: response })); return singular ? response.pop() : response; }; -Thumbs.associate = async function (id, relativePath) { +Thumbs.associate = async function ({ id, path: relativePath, url }) { // Associates a newly uploaded file as a thumb to the passed-in draft or topic const isDraft = validator.isUUID(String(id)); + const value = relativePath || url; const set = `${isDraft ? 'draft' : 'topic'}:${id}:thumbs`; const numThumbs = await db.sortedSetCard(set); - relativePath = relativePath.replace(nconf.get('upload_path'), ''); - db.sortedSetAdd(set, numThumbs, relativePath); - // Associate thumbnails with the main pid - if (!isDraft) { + // Normalize the path to allow for changes in upload_path (and so upload_url can be appended if needed) + if (relativePath) { + relativePath = relativePath.replace(nconf.get('upload_path'), ''); + } + + db.sortedSetAdd(set, numThumbs, value); + + // Associate thumbnails with the main pid (only on local upload) + if (!isDraft && relativePath) { const topics = require('.'); const mainPid = (await topics.getMainPids([id]))[0]; posts.uploads.associate(mainPid, relativePath.replace('/files/', '')); @@ -59,7 +65,7 @@ Thumbs.migrate = async function (uuid, id) { // Converts the draft thumb zset to the topic zset (combines thumbs if applicable) const set = `draft:${uuid}:thumbs`; const thumbs = await db.getSortedSetRange(set, 0, -1); - await Promise.all(thumbs.map(async path => await Thumbs.associate(id, path))); + await Promise.all(thumbs.map(async path => await Thumbs.associate({ id, path }))); await db.delete(set); }; diff --git a/test/topicThumbs.js b/test/topicThumbs.js index 58574638fc..6d2d249801 100644 --- a/test/topicThumbs.js +++ b/test/topicThumbs.js @@ -13,6 +13,7 @@ const user = require('../src/user'); const groups = require('../src/groups'); const topics = require('../src/topics'); const categories = require('../src/categories'); +const plugins = require('../src/plugins'); const file = require('../src/file'); const utils = require('../src/utils'); @@ -27,7 +28,7 @@ describe('Topic thumbs', () => { let fooJar; let fooCSRF; let fooUid; - const thumbPaths = ['files/test.png', 'files/test2.png']; + const thumbPaths = ['files/test.png', 'files/test2.png', 'https://example.org']; const uuid = utils.generateUUID(); function createFiles() { @@ -106,18 +107,34 @@ describe('Topic thumbs', () => { describe('.associate()', () => { it('should add an uploaded file to a zset', async () => { - await topics.thumbs.associate(2, thumbPaths[0]); + await topics.thumbs.associate({ + id: 2, + path: thumbPaths[0], + }); const exists = await db.isSortedSetMember(`topic:2:thumbs`, thumbPaths[0]); assert(exists); }); it('should also work with UUIDs', async () => { - await topics.thumbs.associate(uuid, thumbPaths[1]); + await topics.thumbs.associate({ + id: uuid, + path: thumbPaths[1], + }); const exists = await db.isSortedSetMember(`draft:${uuid}:thumbs`, thumbPaths[1]); assert(exists); }); + + it('should also work with a URL', async () => { + await topics.thumbs.associate({ + id: 2, + path: thumbPaths[2], + }); + + const exists = await db.isSortedSetMember(`topic:2:thumbs`, thumbPaths[2]); + assert(exists); + }); }); describe('.migrate()', () => { @@ -125,13 +142,18 @@ describe('Topic thumbs', () => { await topics.thumbs.migrate(uuid, 2); const thumbs = await topics.thumbs.get(2); - assert.strictEqual(thumbs.length, 2); + assert.strictEqual(thumbs.length, 3); assert.deepStrictEqual(thumbs, [ { id: 2, name: 'test.png', url: `${nconf.get('upload_url')}/${thumbPaths[0]}`, }, + { + id: 2, + name: 'example.org', + url: 'https://example.org', + }, { id: 2, name: 'test2.png', @@ -143,7 +165,10 @@ describe('Topic thumbs', () => { describe(`.delete()`, () => { it('should remove a file from sorted set AND disk', async () => { - await topics.thumbs.associate(1, thumbPaths[0]); + await topics.thumbs.associate({ + id: 1, + path: thumbPaths[0], + }); await topics.thumbs.delete(1, thumbPaths[0]); assert.strictEqual(await db.isSortedSetMember('topic:1:thumbs', thumbPaths[0]), false); @@ -151,13 +176,26 @@ describe('Topic thumbs', () => { }); it('should also work with UUIDs', async () => { - await topics.thumbs.associate(uuid, thumbPaths[1]); + await topics.thumbs.associate({ + id: uuid, + path: thumbPaths[1], + }); await topics.thumbs.delete(uuid, thumbPaths[1]); assert.strictEqual(await db.isSortedSetMember(`draft:${uuid}:thumbs`, thumbPaths[1]), false); assert.strictEqual(await file.exists(`${nconf.get('upload_path')}/${thumbPaths[1]}`), false); }); + it('should also work with URLs', async () => { + await topics.thumbs.associate({ + id: uuid, + path: thumbPaths[2], + }); + await topics.thumbs.delete(uuid, thumbPaths[2]); + + assert.strictEqual(await db.isSortedSetMember(`draft:${uuid}:thumbs`, thumbPaths[2]), false); + }); + it('should not delete the file from disk if not associated with the tid', async () => { createFiles(); await topics.thumbs.delete(uuid, thumbPaths[0]); @@ -186,6 +224,27 @@ describe('Topic thumbs', () => { }); }); + it('should succeed with uploader plugins', async () => { + const hookMethod = async () => ({ + name: 'test.png', + url: 'https://example.org', + }); + await plugins.hooks.register('test', { + hook: 'filter:uploadFile', + method: hookMethod, + }); + + await new Promise((resolve) => { + helpers.uploadFile(`${nconf.get('url')}/api/v3/topics/${uuid}/thumbs`, path.join(__dirname, './files/test.png'), {}, adminJar, adminCSRF, function (err, res, body) { + assert.ifError(err); + assert.strictEqual(res.statusCode, 200); + resolve(); + }); + }); + + await plugins.hooks.unregister('test', 'filter:uploadFile', hookMethod); + }); + it('should fail with a non-existant tid', (done) => { helpers.uploadFile(`${nconf.get('url')}/api/v3/topics/2/thumbs`, path.join(__dirname, './files/test.png'), {}, adminJar, adminCSRF, function (err, res, body) { assert.ifError(err);