From 1f1e0555092fc19307336f23560a584d88ad34f3 Mon Sep 17 00:00:00 2001
From: Julian Lam <julian@nodebb.org>
Date: Tue, 17 Apr 2018 13:31:54 -0400
Subject: [PATCH] updated post association code to only associate files that
 exist, closes #6455

---
 src/posts/uploads.js | 24 +++++++++++++++++++-----
 test/posts.js        | 20 +++++++++++++++++++-
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/src/posts/uploads.js b/src/posts/uploads.js
index a84088ecc4..496ff9e068 100644
--- a/src/posts/uploads.js
+++ b/src/posts/uploads.js
@@ -2,6 +2,8 @@
 
 var async = require('async');
 var crypto = require('crypto');
+var fs = require('fs');
+var path = require('path');
 
 var db = require('../database');
 
@@ -9,6 +11,7 @@ module.exports = function (Posts) {
 	Posts.uploads = {};
 
 	const md5 = filename => crypto.createHash('md5').update(filename).digest('hex');
+	const pathPrefix = path.join(__dirname, '../../public/uploads/files');
 
 	Posts.uploads.sync = function (pid, callback) {
 		// Scans a post and updates sorted set of uploads
@@ -73,12 +76,23 @@ module.exports = function (Posts) {
 		const now = Date.now();
 		filePaths = !Array.isArray(filePaths) ? [filePaths] : filePaths;
 		const scores = filePaths.map(() => now);
-		let methods = [async.apply(db.sortedSetAdd.bind(db), 'post:' + pid + ':uploads', scores, filePaths)];
-		methods = methods.concat(filePaths.map(path => async.apply(db.sortedSetAdd.bind(db), 'upload:' + md5(path) + ':pids', now, pid)));
 
-		async.parallel(methods, function (err) {
-			// Strictly return only err
-			callback(err);
+		async.filter(filePaths, function (filePath, next) {
+			// Only process files that exist
+			fs.access(path.join(pathPrefix, filePath), fs.constants.F_OK | fs.constants.R_OK, function (err) {
+				next(null, !err);
+			});
+		}, function (err, filePaths) {
+			let methods = [async.apply(db.sortedSetAdd.bind(db), 'post:' + pid + ':uploads', scores, filePaths)];
+			if (err) {
+				return callback(err);
+			}
+
+			methods = methods.concat(filePaths.map(path => async.apply(db.sortedSetAdd.bind(db), 'upload:' + md5(path) + ':pids', now, pid)));
+			async.parallel(methods, function (err) {
+				// Strictly return only err
+				callback(err);
+			});
 		});
 	};
 
diff --git a/test/posts.js b/test/posts.js
index 80d3f9614a..f6b2c28261 100644
--- a/test/posts.js
+++ b/test/posts.js
@@ -6,6 +6,8 @@ var async = require('async');
 var request = require('request');
 var nconf = require('nconf');
 var crypto = require('crypto');
+var fs = require('fs');
+var path = require('path');
 
 var db = require('./mocks/databasemock');
 var topics = require('../src/topics');
@@ -883,6 +885,10 @@ describe('Post\'s', function () {
 		var pid;
 
 		before(function (done) {
+			// Create stub files for testing
+			['abracadabra.png', 'shazam.jpg', 'whoa.gif', 'amazeballs.jpg', 'wut.txt', 'test.bmp']
+				.forEach(filename => fs.closeSync(fs.openSync(path.join(__dirname, '../public/uploads/files', filename), 'w')));
+
 			topics.post({
 				uid: 1,
 				cid: 1,
@@ -1001,6 +1007,18 @@ describe('Post\'s', function () {
 					done();
 				});
 			});
+
+			it('should not associate a file that does not exist on the local disk', function (done) {
+				async.waterfall([
+					async.apply(posts.uploads.associate, pid, ['nonexistant.xls']),
+					async.apply(posts.uploads.list, pid),
+				], function (err, uploads) {
+					assert.ifError(err);
+					assert.strictEqual(uploads.length, 5);
+					assert.strictEqual(false, uploads.includes('nonexistant.xls'));
+					done();
+				});
+			});
 		});
 
 		describe('.dissociate()', function () {
@@ -1046,7 +1064,7 @@ describe('Post\'s', function () {
 					uid: 1,
 					tid: topicPostData.topicData.tid,
 					timestamp: Date.now(),
-					content: '[abcdef](/assets/uploads/files/shazam.png)',
+					content: '[abcdef](/assets/uploads/files/shazam.jpg)',
 				}, function (err, replyData) {
 					assert.ifError(err);
 					topic = topicPostData;