From aaacdb8413c9bc3f3d47c249c0d0287eac1cad88 Mon Sep 17 00:00:00 2001 From: Peter Jaszkowiak Date: Wed, 8 Feb 2017 11:41:24 -0700 Subject: [PATCH] Fix #5416, uploads path config setting - Finish moving uploads route to `/assets/uploads` - Remove `upload_url` config setting, it was broken --- app.js | 3 +++ public/src/app.js | 2 +- src/controllers/admin/uploads.js | 4 ++-- src/controllers/index.js | 12 +++++------ src/file.js | 29 ++++++++++++++----------- src/groups/cover.js | 2 +- src/meta/sounds.js | 37 ++++++++++++++++++++++---------- src/meta/tags.js | 12 +++++------ src/middleware/index.js | 2 +- src/routes/index.js | 5 +++++ src/socket.io/user/picture.js | 2 +- src/start.js | 4 ---- src/topics/thumb.js | 4 ++-- test/mocks/databasemock.js | 7 ++---- test/uploads.js | 10 ++++----- test/user.js | 2 +- 16 files changed, 79 insertions(+), 58 deletions(-) diff --git a/app.js b/app.js index 77da25936d..9d99f09156 100644 --- 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 index 00d5e7c017..0006db25d5 100644 --- 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 index 25b2ae34fd..346e7765e4 100644 --- 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 index 7b97c878d6..11ea6c8e95 100644 --- 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 index d8fa128e5e..5e13c5b2a9 100644 --- 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 index 65ff0368cb..3512a235d5 100644 --- 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 index e44f43cccc..d237a51273 100644 --- a/src/meta/sounds.js +++ b/src/meta/sounds.js @@ -32,11 +32,23 @@ 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 @@ -44,15 +56,9 @@ module.exports = function (Meta) { 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 index 1d64a7f93d..f7e349f353 100644 --- 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 index c36f5767ed..105142d15e 100644 --- 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 index 0b9e6fce9a..84e0ce5301 100644 --- 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 index eacc5197aa..deceb70f8a 100644 --- 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 index d314596036..0a8ed18e67 100644 --- 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 index 7d22365293..a5ca38ed3e 100644 --- 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 index fa17493ae2..db28b8025c 100644 --- 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 index 0938555f65..43e39d2f26 100644 --- 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 index b0ed74887b..43b5b3f847 100644 --- 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(); });