From b26e9b5993818225e26bb5843eed5255cd190894 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?= Date: Tue, 20 Oct 2020 00:24:34 -0400 Subject: [PATCH] fix: #8595, dont save escaped data when renaming groups --- src/groups/update.js | 2 +- src/navigation/admin.js | 26 ++++--- test/groups.js | 148 ++++++++++++++++++---------------------- 3 files changed, 86 insertions(+), 90 deletions(-) diff --git a/src/groups/update.js b/src/groups/update.js index be2a04fcd1..e83c016b82 100644 --- a/src/groups/update.js +++ b/src/groups/update.js @@ -232,7 +232,7 @@ module.exports = function (Groups) { navItem.groups.splice(navItem.groups.indexOf(oldName), 1, newName); } }); - + navigation.unescapeFields(navItems); await navigation.save(navItems); } diff --git a/src/navigation/admin.js b/src/navigation/admin.js index 499fd9c5ee..b139660755 100644 --- a/src/navigation/admin.js +++ b/src/navigation/admin.js @@ -35,27 +35,37 @@ admin.getAdmin = async function () { return { enabled: enabled, available: available }; }; +const fieldsToEscape = ['iconClass', 'class', 'route', 'id', 'text', 'textClass', 'title']; + +admin.escapeFields = navItems => toggleEscape(navItems, true); +admin.unescapeFields = navItems => toggleEscape(navItems, false); + +function toggleEscape(navItems, flag) { + navItems.forEach(function (item) { + if (item) { + fieldsToEscape.forEach((field) => { + if (item.hasOwnProperty(field)) { + item[field] = validator[flag ? 'escape' : 'unescape'](String(item[field])); + } + }); + } + }); +} + admin.get = async function () { if (cache) { return _.cloneDeep(cache); } const data = await db.getSortedSetRange('navigation:enabled', 0, -1); - const escapeFields = ['iconClass', 'class', 'route', 'id', 'text', 'textClass', 'title']; cache = data.map(function (item) { item = JSON.parse(item); - - escapeFields.forEach((field) => { - if (item.hasOwnProperty(field)) { - item[field] = validator.escape(String(item[field])); - } - }); - item.groups = item.groups || []; if (item.groups && !Array.isArray(item.groups)) { item.groups = [item.groups]; } return item; }); + admin.escapeFields(cache); return _.cloneDeep(cache); }; diff --git a/test/groups.js b/test/groups.js index 3b2dee0356..468a811687 100644 --- a/test/groups.js +++ b/test/groups.js @@ -11,81 +11,67 @@ var Groups = require('../src/groups'); var User = require('../src/user'); var socketGroups = require('../src/socket.io/groups'); var meta = require('../src/meta'); +var navigation = require('../src/navigation/admin'); + describe('Groups', function () { var adminUid; var testUid; - before(function (done) { - async.series([ - function (next) { - // Create a group to play around with - Groups.create({ - name: 'Test', - description: 'Foobar!', - }, next); - }, - function (next) { - Groups.create({ - name: 'PrivateNoJoin', - description: 'Private group', - private: 1, - disableJoinRequests: 1, - }, next); - }, - function (next) { - Groups.create({ - name: 'PrivateCanJoin', - description: 'Private group', - private: 1, - disableJoinRequests: 0, - }, next); - }, - async () => { - await Groups.create({ - name: 'PrivateNoLeave', - description: 'Private group', - private: 1, - disableLeave: 1, - }); - }, - async () => { - await Groups.create({ - name: 'Global Moderators', - userTitle: 'Global Moderator', - description: 'Forum wide moderators', - hidden: 0, - private: 1, - disableJoinRequests: 1, - }); - }, - function (next) { - // Create a new user - User.create({ - username: 'testuser', - email: 'b@c.com', - }, next); - }, - function (next) { - User.create({ - username: 'admin', - email: 'admin@admin.com', - password: '123456', - }, next); - }, - function (next) { - // Also create a hidden group - Groups.join('Hidden', 'Test', next); - }, - function (next) { - // create another group that starts with test for search/sort - Groups.create({ name: 'Test2', description: 'Foobar!' }, next); - }, - ], function (err, results) { - assert.ifError(err); - testUid = results[5]; - adminUid = results[6]; - Groups.join('administrators', adminUid, done); + before(async function () { + const navData = require('../install/data/navigation.json'); + await navigation.save(navData); + + await Groups.create({ + name: 'Test', + description: 'Foobar!', + }); + + await Groups.create({ + name: 'PrivateNoJoin', + description: 'Private group', + private: 1, + disableJoinRequests: 1, + }); + + await Groups.create({ + name: 'PrivateCanJoin', + description: 'Private group', + private: 1, + disableJoinRequests: 0, + }); + + await Groups.create({ + name: 'PrivateNoLeave', + description: 'Private group', + private: 1, + disableLeave: 1, + }); + + await Groups.create({ + name: 'Global Moderators', + userTitle: 'Global Moderator', + description: 'Forum wide moderators', + hidden: 0, + private: 1, + disableJoinRequests: 1, }); + + // Also create a hidden group + await Groups.join('Hidden', 'Test'); + // create another group that starts with test for search/sort + await Groups.create({ name: 'Test2', description: 'Foobar!' }); + + testUid = await User.create({ + username: 'testuser', + email: 'b@c.com', + }); + + adminUid = await User.create({ + username: 'admin', + email: 'admin@admin.com', + password: '123456', + }); + await Groups.join('administrators', adminUid); }); describe('.list()', function () { @@ -457,19 +443,19 @@ describe('Groups', function () { }); }); - it('should rename a group if the name was updated', function (done) { - Groups.update('updateTestGroup', { + it('should rename a group and not break navigation routes', async function () { + await Groups.update('updateTestGroup', { name: 'updateTestGroup?', - }, function (err) { - assert.ifError(err); - - Groups.get('updateTestGroup?', {}, function (err, groupObj) { - assert.ifError(err); - assert.strictEqual('updateTestGroup?', groupObj.name); - assert.strictEqual('updatetestgroup', groupObj.slug); - done(); - }); }); + + const groupObj = await Groups.get('updateTestGroup?', {}); + assert.strictEqual('updateTestGroup?', groupObj.name); + assert.strictEqual('updatetestgroup', groupObj.slug); + + const navigation = require('../src/navigation/admin'); + const navItems = await navigation.get(); + assert.strictEqual(navItems[0].route, '/categories'); + console.log(navItems[0]); }); it('should fail if system groups is being renamed', function (done) {