Skip to content

Commit

Permalink
fix: #9092, Topic thumbnails do not work with third-party uploaders
Browse files Browse the repository at this point in the history
  • Loading branch information
@julianlam
julianlam committed Dec 9, 2020
1 parent dd448e2 commit 3e54b70
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 15 deletions.
14 changes: 13 additions & 1 deletion src/controllers/write/topics.js
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
});
}));
}
};
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/routes/write/topics.js
View file
Expand Up @@ -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;
};
20 changes: 13 additions & 7 deletions src/topics/thumbs.js
View file
Expand Up @@ -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/', ''));
Expand All @@ -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);
};

Expand Down
71 changes: 65 additions & 6 deletions test/topicThumbs.js
View file
Expand Up @@ -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');

Expand All @@ -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() {
Expand Down Expand Up @@ -106,32 +107,53 @@ 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()', () => {
it('should combine the thumbs uploaded to a UUID zset and combine it with a topic\'s thumb zset', async () => {
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',
Expand All @@ -143,21 +165,37 @@ 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);
assert.strictEqual(await file.exists(`${nconf.get('upload_path')}/${thumbPaths[0]}`), false);
});

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]);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 3e54b70

Please sign in to comment.