Fix #5416, uploads path config setting

- Finish moving uploads route to `/assets/uploads` - Remove `upload_url` config setting, it was broken
NodeBB · Feb 8, 2017 · aaacdb8 · aaacdb8
1 parent d43564d
commit aaacdb8
Show file tree

Hide file tree

Showing 16 changed files with 79 additions and 58 deletions.
diff --git a/app.js b/app.js
@@ -99,6 +99,7 @@ function loadConfig(callback) {
 	nconf.defaults({
 		base_dir: __dirname,
 		themes_path: path.join(__dirname, 'node_modules'),
+		upload_path: 'public/uploads',
 		views_dir: path.join(__dirname, 'build/public/templates'),
 		version: pkg.version
 	});
@@ -112,6 +113,8 @@ function loadConfig(callback) {
 	nconf.set('themes_path', path.resolve(__dirname, nconf.get('themes_path')));
 	nconf.set('core_templates_path', path.join(__dirname, 'src/views'));
 	nconf.set('base_templates_path', path.join(nconf.get('themes_path'), 'nodebb-theme-persona/templates'));
+
+	nconf.set('upload_path', path.resolve(nconf.get('base_dir'), nconf.get('upload_path')));
 
 	if (nconf.get('url')) {
 		nconf.set('url_parsed', url.parse(nconf.get('url')));

diff --git a/public/src/app.js b/public/src/app.js
@@ -558,7 +558,7 @@ app.cacheBuster = null;
 
 		var scriptEl = document.createElement('script');
 		scriptEl.type = 'text/javascript';
-		scriptEl.src = config.relative_path + '/vendor/jquery/js/jquery-ui.js' + '?' + config['cache-buster'];
+		scriptEl.src = config.relative_path + '/assets/vendor/jquery/js/jquery-ui.js' + '?' + config['cache-buster'];
 		scriptEl.onload = callback;
 		document.head.appendChild(scriptEl);
 	};

diff --git a/src/controllers/admin/uploads.js b/src/controllers/admin/uploads.js
@@ -70,7 +70,7 @@ uploadsController.uploadTouchIcon = function (req, res, next) {
 				async.series([
 					async.apply(file.saveFileToLocal, 'touchicon-' + size + '.png', 'system', uploadedFile.path),
 					async.apply(image.resizeImage, {
-						path: path.join(nconf.get('base_dir'), nconf.get('upload_path'), 'system', 'touchicon-' + size + '.png'),
+						path: path.join(nconf.get('upload_path'), 'system', 'touchicon-' + size + '.png'),
 						extension: 'png',
 						width: size,
 						height: size
@@ -106,7 +106,7 @@ uploadsController.uploadSound = function (req, res, next) {
 		}
 
 		var	soundsPath = path.join(__dirname, '../../../build/public/sounds'),
-			filePath = path.join(__dirname, '../../../public/uploads/sounds', uploadedFile.name);
+			filePath = path.join(nconf.get('upload_path'), 'sounds', uploadedFile.name);
 
 		file.link(filePath, path.join(soundsPath, path.basename(filePath)));
 

diff --git a/src/controllers/index.js b/src/controllers/index.js
@@ -290,32 +290,32 @@ Controllers.manifest = function (req, res) {
 
 	if (meta.config['brand:touchIcon']) {
 		manifest.icons.push({
-			src: nconf.get('relative_path') + '/uploads/system/touchicon-36.png',
+			src: nconf.get('relative_path') + '/assets/uploads/system/touchicon-36.png',
 			sizes: '36x36',
 			type: 'image/png',
 			density: 0.75
 		}, {
-			src: nconf.get('relative_path') + '/uploads/system/touchicon-48.png',
+			src: nconf.get('relative_path') + '/assets/uploads/system/touchicon-48.png',
 			sizes: '48x48',
 			type: 'image/png',
 			density: 1.0
 		}, {
-			src: nconf.get('relative_path') + '/uploads/system/touchicon-72.png',
+			src: nconf.get('relative_path') + '/assets/uploads/system/touchicon-72.png',
 			sizes: '72x72',
 			type: 'image/png',
 			density: 1.5
 		}, {
-			src: nconf.get('relative_path') + '/uploads/system/touchicon-96.png',
+			src: nconf.get('relative_path') + '/assets/uploads/system/touchicon-96.png',
 			sizes: '96x96',
 			type: 'image/png',
 			density: 2.0
 		}, {
-			src: nconf.get('relative_path') + '/uploads/system/touchicon-144.png',
+			src: nconf.get('relative_path') + '/assets/uploads/system/touchicon-144.png',
 			sizes: '144x144',
 			type: 'image/png',
 			density: 3.0
 		}, {
-			src: nconf.get('relative_path') + '/uploads/system/touchicon-192.png',
+			src: nconf.get('relative_path') + '/assets/uploads/system/touchicon-192.png',
 			sizes: '192x192',
 			type: 'image/png',
 			density: 4.0

diff --git a/src/file.js b/src/file.js
@@ -5,6 +5,7 @@ var nconf = require('nconf');
 var path = require('path');
 var winston = require('winston');
 var jimp = require('jimp');
+var mkdirp = require('mkdirp');
 
 var utils = require('../public/src/utils');
 
@@ -20,27 +21,31 @@ file.saveFileToLocal = function (filename, folder, tempPath, callback) {
 	});
 	filename = filename.join('.');
 
-	var uploadPath = path.join(nconf.get('base_dir'), nconf.get('upload_path'), folder, filename);
+	var uploadPath = path.join(nconf.get('upload_path'), folder, filename);
 
 	winston.verbose('Saving file ' + filename + ' to : ' + uploadPath);
+	mkdirp(path.dirname(uploadPath), function (err) {
+		if (err) {
+			callback(err);
+		}
 
-	var is = fs.createReadStream(tempPath);
-	var os = fs.createWriteStream(uploadPath);
-	is.on('end', function () {
-		callback(null, {
-			url: nconf.get('upload_url') + '/' + folder + '/' + filename,
-			path: uploadPath
+		var is = fs.createReadStream(tempPath);
+		var os = fs.createWriteStream(uploadPath);
+		is.on('end', function () {
+			callback(null, {
+				url: '/assets/uploads/' + folder + '/' + filename,
+				path: uploadPath
+			});
 		});
-	});
 
-	os.on('error', callback);
-
-	is.pipe(os);
+		os.on('error', callback);
+		is.pipe(os);
+	});
 };
 
 file.base64ToLocal = function (imageData, uploadPath, callback) {
 	var buffer = new Buffer(imageData.slice(imageData.indexOf('base64') + 7), 'base64');
-	uploadPath = path.join(nconf.get('base_dir'), nconf.get('upload_path'), uploadPath);
+	uploadPath = path.join(nconf.get('upload_path'), uploadPath);
 
 	fs.writeFile(uploadPath, buffer, {
 		encoding: 'base64'

diff --git a/src/groups/cover.js b/src/groups/cover.js
@@ -105,7 +105,7 @@ module.exports = function (Groups) {
 		md5sum = md5sum.digest('hex');
 
 		// Save image
-		var tempPath = path.join(nconf.get('base_dir'), nconf.get('upload_path'), md5sum) + '.png';
+		var tempPath = path.join(nconf.get('upload_path'), md5sum + '.png');
 		var buffer = new Buffer(imageData.slice(imageData.indexOf('base64') + 7), 'base64');
 
 		fs.writeFile(tempPath, buffer, {

diff --git a/src/meta/sounds.js b/src/meta/sounds.js
@@ -32,27 +32,33 @@ module.exports = function (Meta) {
 				fs.readdir(path.join(__dirname, '../../build/public/sounds'), next);
 			},
 			function (sounds, next) {
-				fs.readdir(path.join(__dirname, '../../public/uploads/sounds'), function (err, uploaded) {
-					next(err, sounds.concat(uploaded));
+				fs.readdir(path.join(nconf.get('upload_path'), 'sounds'), function (err, uploaded) {
+					if (err) {
+						if (err.code === 'ENOENT') {
+							return next(null, sounds);
+						}
+						return next(err);
+					}
+					next(null, sounds.concat(uploaded));
 				});
 			}
 		], function (err, files) {
+			if (err) {
+				winston.error('Could not get local sound files:' + err.message);
+				console.log(err.stack);
+				return callback(null, []);
+			}
+
 			var	localList = {};
 
 			// Filter out hidden files
 			files = files.filter(function (filename) {
 				return !filename.startsWith('.');
 			});
 
-			if (err) {
-				winston.error('Could not get local sound files:' + err.message);
-				console.log(err.stack);
-				return callback(null, []);
-			}
-
 			// Return proper paths
 			files.forEach(function (filename) {
-				localList[filename] = nconf.get('relative_path') + '/sounds/' + filename;
+				localList[filename] = nconf.get('relative_path') + '/assets/sounds/' + filename;
 			});
 
 			callback(null, localList);
@@ -93,13 +99,22 @@ module.exports = function (Meta) {
 
 		async.waterfall([
 			function (next) {
-				fs.readdir(path.join(__dirname, '../../public/uploads/sounds'), next);
+				fs.readdir(path.join(nconf.get('upload_path'), 'sounds'), function (err, files) {
+					if (err) {
+						if (err.code === 'ENOENT') {
+							return next(null, []);
+						}
+						return next(err);
+					}
+
+					next(null, files);
+				});
 			},
 			function (uploaded, next) {
 				uploaded = uploaded.filter(function (filename) {
 					return !filename.startsWith('.');
 				}).map(function (filename) {
-					return path.join(__dirname, '../../public/uploads/sounds', filename);
+					return path.join(nconf.get('upload_path'), 'sounds', filename);
 				});
 
 				plugins.fireHook('filter:sounds.get', uploaded, function (err, filePaths) {

diff --git a/src/meta/tags.js b/src/meta/tags.js
@@ -60,27 +60,27 @@ module.exports = function (Meta) {
 					}, {
 						rel: 'icon',
 						sizes: '36x36',
-						href: nconf.get('relative_path') + '/uploads/system/touchicon-36.png'
+						href: nconf.get('relative_path') + '/assets/uploads/system/touchicon-36.png'
 					}, {
 						rel: 'icon',
 						sizes: '48x48',
-						href: nconf.get('relative_path') + '/uploads/system/touchicon-48.png'
+						href: nconf.get('relative_path') + '/assets/uploads/system/touchicon-48.png'
 					}, {
 						rel: 'icon',
 						sizes: '72x72',
-						href: nconf.get('relative_path') + '/uploads/system/touchicon-72.png'
+						href: nconf.get('relative_path') + '/assets/uploads/system/touchicon-72.png'
 					}, {
 						rel: 'icon',
 						sizes: '96x96',
-						href: nconf.get('relative_path') + '/uploads/system/touchicon-96.png'
+						href: nconf.get('relative_path') + '/assets/uploads/system/touchicon-96.png'
 					}, {
 						rel: 'icon',
 						sizes: '144x144',
-						href: nconf.get('relative_path') + '/uploads/system/touchicon-144.png'
+						href: nconf.get('relative_path') + '/assets/uploads/system/touchicon-144.png'
 					}, {
 						rel: 'icon',
 						sizes: '192x192',
-						href: nconf.get('relative_path') + '/uploads/system/touchicon-192.png'
+						href: nconf.get('relative_path') + '/assets/uploads/system/touchicon-192.png'
 					});
 				}
 				plugins.fireHook('filter:meta.getLinkTags', defaultLinks, next);

diff --git a/src/middleware/index.js b/src/middleware/index.js
@@ -162,7 +162,7 @@ middleware.privateUploads = function (req, res, next) {
 	if (req.user || parseInt(meta.config.privateUploads, 10) !== 1) {
 		return next();
 	}
-	if (req.path.startsWith('/uploads/files')) {
+	if (req.path.startsWith('/assets/uploads/files')) {
 		return res.status(403).json('not-allowed');
 	}
 	next();

diff --git a/src/routes/index.js b/src/routes/index.js
@@ -144,6 +144,11 @@ module.exports = function (app, middleware, hotswapIds) {
 
 	app.use(middleware.privateUploads);
 
+	if (path.resolve(__dirname, '../../public/uploads') !== nconf.get('upload_path')) {
+		app.use(relativePath + '/assets/uploads', express.static(nconf.get('upload_path'), {
+			maxAge: app.enabled('cache') ? 5184000000 : 0
+		}));
+	}
 	app.use(relativePath + '/assets', express.static(path.join(__dirname, '../../build/public'), {
 		maxAge: app.enabled('cache') ? 5184000000 : 0
 	}));

diff --git a/src/socket.io/user/picture.js b/src/socket.io/user/picture.js
@@ -86,7 +86,7 @@ module.exports = function (SocketUser) {
 			function (userData, next) {
 				if (userData.uploadedpicture && !userData.uploadedpicture.startsWith('http')) {
 					var pathToFile = path.join(nconf.get('base_dir'), 'public', userData.uploadedpicture);
-					if (pathToFile.startsWith(path.join(nconf.get('base_dir'), nconf.get('upload_path')))) {
+					if (pathToFile.startsWith(nconf.get('upload_path'))) {
 						require('fs').unlink(pathToFile, function (err) {
 							if (err) {
 								winston.error(err);

diff --git a/src/start.js b/src/start.js
@@ -88,9 +88,6 @@ start.start = function () {
 
 function setupConfigs() {
 	// nconf defaults, if not set in config
-	if (!nconf.get('upload_path')) {
-		nconf.set('upload_path', '/public/uploads');
-	}
 	if (!nconf.get('sessionKey')) {
 		nconf.set('sessionKey', 'express.sid');
 	}
@@ -102,7 +99,6 @@ function setupConfigs() {
 	nconf.set('use_port', !!urlObject.port);
 	nconf.set('relative_path', relativePath);
 	nconf.set('port', urlObject.port || nconf.get('port') || nconf.get('PORT') || (nconf.get('PORT_ENV_VAR') ? nconf.get(nconf.get('PORT_ENV_VAR')) : false) || 4567);
-	nconf.set('upload_url', nconf.get('upload_path').replace(/^\/public/, ''));
 }
 
 function printStartupInfo() {

diff --git a/src/topics/thumb.js b/src/topics/thumb.js
@@ -41,7 +41,7 @@ module.exports = function (Topics) {
 					extension = '.' + mime.extension(type);
 				}
 				filename = Date.now() + '-topic-thumb' + extension;
-				pathToUpload = path.join(nconf.get('base_dir'), nconf.get('upload_path'), 'files', filename);
+				pathToUpload = path.join(nconf.get('upload_path'), 'files', filename);
 
 				request(data.thumb).pipe(fs.createWriteStream(pathToUpload)).on('close', next);
 			},
@@ -59,7 +59,7 @@ module.exports = function (Topics) {
 			},
 			function (next) {
 				if (!plugins.hasListeners('filter:uploadImage')) {
-					data.thumb = path.join(nconf.get('upload_url'), 'files', filename);
+					data.thumb = '/assets/uploads/files/' + filename;
 					return callback();
 				}
 

diff --git a/test/mocks/databasemock.js b/test/mocks/databasemock.js
@@ -19,7 +19,7 @@
 	nconf.defaults({
 		base_dir: path.join(__dirname,'../..'),
 		themes_path: path.join(__dirname, '../../node_modules'),
-		upload_url: path.join(path.sep, '../../uploads', path.sep),
+		upload_path: 'public/uploads',
 		views_dir: path.join(__dirname, '../../build/public/templates'),
 		relative_path: ''
 	});
@@ -121,9 +121,6 @@
 			},
 			function (next) {
 				// nconf defaults, if not set in config
-				if (!nconf.get('upload_path')) {
-					nconf.set('upload_path', '/public/uploads');
-				}
 				if (!nconf.get('sessionKey')) {
 					nconf.set('sessionKey', 'express.sid');
 				}
@@ -135,7 +132,7 @@
 				nconf.set('use_port', !!urlObject.port);
 				nconf.set('relative_path', relativePath);
 				nconf.set('port', urlObject.port || nconf.get('port') || nconf.get('PORT') || (nconf.get('PORT_ENV_VAR') ? nconf.get(nconf.get('PORT_ENV_VAR')) : false) || 4567);
-				nconf.set('upload_url', nconf.get('upload_path').replace(/^\/public/, ''));
+				nconf.set('upload_path', path.join(nconf.get('base_dir'), nconf.get('upload_path')));
 
 				nconf.set('core_templates_path', path.join(__dirname, '../../src/views'));
 				nconf.set('base_templates_path', path.join(nconf.get('themes_path'), 'nodebb-theme-persona/templates'));

diff --git a/test/uploads.js b/test/uploads.js
@@ -72,7 +72,7 @@ describe('Upload Controllers', function () {
 				assert.equal(res.statusCode, 200);
 				assert(Array.isArray(body));
 				assert.equal(body.length, 1);
-				assert.equal(body[0].url, '/uploads/profile/' + regularUid + '-profileimg.png');
+				assert.equal(body[0].url, '/assets/uploads/profile/' + regularUid + '-profileimg.png');
 				done();
 			});
 		});
@@ -122,7 +122,7 @@ describe('Upload Controllers', function () {
 				assert.ifError(err);
 				assert.equal(res.statusCode, 200);
 				assert(Array.isArray(body));
-				assert.equal(body[0].url, '/uploads/system/site-logo.png');
+				assert.equal(body[0].url, '/assets/uploads/system/site-logo.png');
 				done();
 			});
 		});
@@ -132,7 +132,7 @@ describe('Upload Controllers', function () {
 				assert.ifError(err);
 				assert.equal(res.statusCode, 200);
 				assert(Array.isArray(body));
-				assert.equal(body[0].url, '/uploads/category/category-1.png');
+				assert.equal(body[0].url, '/assets/uploads/category/category-1.png');
 				done();
 			});
 		});
@@ -142,7 +142,7 @@ describe('Upload Controllers', function () {
 				assert.ifError(err);
 				assert.equal(res.statusCode, 200);
 				assert(Array.isArray(body));
-				assert.equal(body[0].url, '/uploads/system/favicon.ico');
+				assert.equal(body[0].url, '/assets/uploads/system/favicon.ico');
 				done();
 			});
 		});
@@ -152,7 +152,7 @@ describe('Upload Controllers', function () {
 				assert.ifError(err);
 				assert.equal(res.statusCode, 200);
 				assert(Array.isArray(body));
-				assert.equal(body[0].url, '/uploads/system/touchicon-orig.png');
+				assert.equal(body[0].url, '/assets/uploads/system/touchicon-orig.png');
 				done();
 			});
 		});

diff --git a/test/user.js b/test/user.js
@@ -503,7 +503,7 @@ describe('User', function () {
 			};
 			User.uploadPicture(uid, picture, function (err, uploadedPicture) {
 				assert.ifError(err);
-				assert.equal(uploadedPicture.url, '/uploads/profile/' + uid + '-profileimg.png');
+				assert.equal(uploadedPicture.url, '/assets/uploads/profile/' + uid + '-profileimg.png');
 				assert.equal(uploadedPicture.path, path.join(nconf.get('base_dir'), 'public', 'uploads', 'profile', uid + '-profileimg.png'));
 				done();
 			});