Merge branch 'leipert-simplify-emoji-sanitization' into 'master'

Improvements to Emoji sanitization

See merge request gitlab-org/gitlab!75011

app/assets/javascripts/behaviors/gl_emoji.js

@@ -33,7 +33,7 @@ class GlEmoji extends HTMLElement {
           this.dataset.unicodeVersion = unicodeVersion;
 
           emojiUnicode = emojiInfo.e;
-          this.innerHTML = emojiInfo.e;
+          this.textContent = emojiInfo.e;
 
           this.title = emojiInfo.d;
         }

app/assets/javascripts/emoji/constants.js

@@ -16,3 +16,6 @@ export const CATEGORY_ICON_MAP = {
 export const EMOJIS_PER_ROW = 9;
 export const EMOJI_ROW_HEIGHT = 34;
 export const CATEGORY_ROW_HEIGHT = 37;
+
+export const CACHE_VERSION_KEY = 'gl-emoji-map-version';
+export const CACHE_KEY = 'gl-emoji-map';

app/assets/javascripts/emoji/index.js

 import { escape, minBy } from 'lodash';
+import emojiRegexFactory from 'emoji-regex';
 import emojiAliases from 'emojis/aliases.json';
-import { sanitize } from '~/lib/dompurify';
 import AccessorUtilities from '../lib/utils/accessor';
 import axios from '../lib/utils/axios_utils';
-import { CATEGORY_ICON_MAP, FREQUENTLY_USED_KEY } from './constants';
+import { CACHE_KEY, CACHE_VERSION_KEY, CATEGORY_ICON_MAP, FREQUENTLY_USED_KEY } from './constants';
 
 let emojiMap = null;
 let validEmojiNames = null;
@@ -17,10 +17,15 @@ const isLocalStorageAvailable = AccessorUtilities.canUseLocalStorage();
 async function loadEmoji() {
   if (
     isLocalStorageAvailable &&
-    window.localStorage.getItem('gl-emoji-map-version') === EMOJI_VERSION &&
-    window.localStorage.getItem('gl-emoji-map')
+    window.localStorage.getItem(CACHE_VERSION_KEY) === EMOJI_VERSION &&
+    window.localStorage.getItem(CACHE_KEY)
   ) {
-    return JSON.parse(window.localStorage.getItem('gl-emoji-map'));
+    const emojis = JSON.parse(window.localStorage.getItem(CACHE_KEY));
+    // Workaround because the pride flag is broken in EMOJI_VERSION = '1'
+    if (emojis.gay_pride_flag) {
+      emojis.gay_pride_flag.e = '🏳️‍🌈';
+    }
+    return emojis;
   }
 
   // We load the JSON file direct from the server
@@ -29,15 +34,19 @@ async function loadEmoji() {
   const { data } = await axios.get(
     `${gon.relative_url_root || ''}/-/emojis/${EMOJI_VERSION}/emojis.json`,
   );
-  window.localStorage.setItem('gl-emoji-map-version', EMOJI_VERSION);
-  window.localStorage.setItem('gl-emoji-map', JSON.stringify(data));
+  window.localStorage.setItem(CACHE_VERSION_KEY, EMOJI_VERSION);
+  window.localStorage.setItem(CACHE_KEY, JSON.stringify(data));
   return data;
 }
 
 async function loadEmojiWithNames() {
-  return Object.entries(await loadEmoji()).reduce((acc, [key, value]) => {
-    acc[key] = { ...value, name: key, e: sanitize(value.e) };
+  const emojiRegex = emojiRegexFactory();
 
+  return Object.entries(await loadEmoji()).reduce((acc, [key, value]) => {
+    // Filter out entries which aren't emojis
+    if (value.e.match(emojiRegex)?.[0] === value.e) {
+      acc[key] = { ...value, name: key };
+    }
     return acc;
   }, {});
 }

package.json

@@ -129,7 +129,7 @@
     "dompurify": "^2.3.4",
     "dropzone": "^4.2.0",
     "editorconfig": "^0.15.3",
-    "emoji-regex": "^7.0.3",
+    "emoji-regex": "^10.0.0",
     "fast-mersenne-twister": "1.0.2",
     "file-loader": "^6.2.0",
     "fuzzaldrin-plus": "^0.6.0",

spec/frontend/__helpers__/emoji.js

-import MockAdapter from 'axios-mock-adapter';
 import { initEmojiMap, EMOJI_VERSION } from '~/emoji';
-import axios from '~/lib/utils/axios_utils';
+import { CACHE_VERSION_KEY, CACHE_KEY } from '~/emoji/constants';
 
-export const emojiFixtureMap = {
+export const validEmoji = {
   atom: {
     moji: '⚛',
     description: 'atom symbol',
@@ -49,11 +48,39 @@ export const emojiFixtureMap = {
     unicodeVersion: '5.1',
     description: 'white medium star',
   },
+  gay_pride_flag: {
+    moji: '🏳️‍🌈',
+    unicodeVersion: '7.0',
+    description: 'because it contains a zero width joiner',
+  },
+  family_mmb: {
+    moji: '👨‍👨‍👦',
+    unicodeVersion: '6.0',
+    description: 'because it contains multiple zero width joiners',
+  },
+};
+
+export const invalidEmoji = {
   xss: {
     moji: '<img src=x onerror=prompt(1)>',
     unicodeVersion: '5.1',
     description: 'xss',
   },
+  non_moji: {
+    moji: 'I am not an emoji...',
+    unicodeVersion: '9.0',
+    description: '...and should be filtered out',
+  },
+  multiple_moji: {
+    moji: '🍂🏭',
+    unicodeVersion: '9.0',
+    description: 'Multiple separate emoji that are not joined by a zero width joiner',
+  },
+};
+
+export const emojiFixtureMap = {
+  ...validEmoji,
+  ...invalidEmoji,
 };
 
 export const mockEmojiData = Object.keys(emojiFixtureMap).reduce((acc, k) => {
@@ -63,11 +90,14 @@ export const mockEmojiData = Object.keys(emojiFixtureMap).reduce((acc, k) => {
   return acc;
 }, {});
 
-export async function initEmojiMock(mockData = mockEmojiData) {
-  const mock = new MockAdapter(axios);
-  mock.onGet(`/-/emojis/${EMOJI_VERSION}/emojis.json`).reply(200, JSON.stringify(mockData));
+export function clearEmojiMock() {
+  localStorage.clear();
+  initEmojiMap.promise = null;
+}
 
+export async function initEmojiMock(mockData = mockEmojiData) {
+  clearEmojiMock();
+  localStorage.setItem(CACHE_VERSION_KEY, EMOJI_VERSION);
+  localStorage.setItem(CACHE_KEY, JSON.stringify(mockData));
   await initEmojiMap();
-
-  return mock;
 }

spec/frontend/awards_handler_spec.js

-import MockAdapter from 'axios-mock-adapter';
 import $ from 'jquery';
 import Cookies from 'js-cookie';
+import { initEmojiMock, clearEmojiMock } from 'helpers/emoji';
 import { useFakeRequestAnimationFrame } from 'helpers/fake_request_animation_frame';
 import loadAwardsHandler from '~/awards_handler';
-import { EMOJI_VERSION } from '~/emoji';
-import axios from '~/lib/utils/axios_utils';
 
 window.gl = window.gl || {};
 window.gon = window.gon || {};
 
-let mock;
 let awardsHandler = null;
 const urlRoot = gon.relative_url_root;
 
@@ -76,8 +73,7 @@ describe('AwardsHandler', () => {
   };
 
   beforeEach(async () => {
-    mock = new MockAdapter(axios);
-    mock.onGet(`/-/emojis/${EMOJI_VERSION}/emojis.json`).reply(200, emojiData);
+    await initEmojiMock(emojiData);
 
     loadFixtures('snippets/show.html');
 
@@ -89,7 +85,7 @@ describe('AwardsHandler', () => {
     // restore original url root value
     gon.relative_url_root = urlRoot;
 
-    mock.restore();
+    clearEmojiMock();
 
     // Undo what we did to the shared <body>
     $('body').removeAttr('data-page');

spec/frontend/behaviors/gl_emoji_spec.js

-import MockAdapter from 'axios-mock-adapter';
+import { initEmojiMock, clearEmojiMock } from 'helpers/emoji';
 import waitForPromises from 'helpers/wait_for_promises';
 import installGlEmojiElement from '~/behaviors/gl_emoji';
-import { initEmojiMap, EMOJI_VERSION } from '~/emoji';
+import { EMOJI_VERSION } from '~/emoji';
 
 import * as EmojiUnicodeSupport from '~/emoji/support';
-import axios from '~/lib/utils/axios_utils';
 
 jest.mock('~/emoji/support');
 
 describe('gl_emoji', () => {
-  let mock;
   const emojiData = {
     grey_question: {
       c: 'symbols',
@@ -38,15 +36,12 @@ describe('gl_emoji', () => {
     return div.firstElementChild;
   }
 
-  beforeEach(() => {
-    mock = new MockAdapter(axios);
-    mock.onGet(`/-/emojis/${EMOJI_VERSION}/emojis.json`).reply(200, emojiData);
-
-    return initEmojiMap().catch(() => {});
+  beforeEach(async () => {
+    await initEmojiMock(emojiData);
   });
 
   afterEach(() => {
-    mock.restore();
+    clearEmojiMock();
 
     document.body.innerHTML = '';
   });

spec/frontend/emoji/index_spec.js

-import { emojiFixtureMap, mockEmojiData, initEmojiMock } from 'helpers/emoji';
+import {
+  emojiFixtureMap,
+  mockEmojiData,
+  initEmojiMock,
+  validEmoji,
+  invalidEmoji,
+  clearEmojiMock,
+} from 'helpers/emoji';
 import { trimText } from 'helpers/text_helper';
-import { glEmojiTag, searchEmoji, getEmojiInfo, sortEmoji } from '~/emoji';
+import {
+  glEmojiTag,
+  searchEmoji,
+  getEmojiInfo,
+  sortEmoji,
+  initEmojiMap,
+  getAllEmoji,
+} from '~/emoji';
+
 import isEmojiUnicodeSupported, {
   isFlagEmoji,
   isRainbowFlagEmoji,
@@ -9,7 +24,6 @@ import isEmojiUnicodeSupported, {
   isHorceRacingSkinToneComboEmoji,
   isPersonZwjEmoji,
 } from '~/emoji/support/is_emoji_unicode_supported';
-import { sanitize } from '~/lib/dompurify';
 
 const emptySupportMap = {
   personZwj: false,
@@ -31,14 +45,55 @@ const emptySupportMap = {
 };
 
 describe('emoji', () => {
-  let mock;
-
   beforeEach(async () => {
-    mock = await initEmojiMock();
+    await initEmojiMock();
   });
 
   afterEach(() => {
-    mock.restore();
+    clearEmojiMock();
+  });
+
+  describe('initEmojiMap', () => {
+    it('should contain valid emoji', async () => {
+      await initEmojiMap();
+
+      const allEmoji = Object.keys(getAllEmoji());
+      Object.keys(validEmoji).forEach((key) => {
+        expect(allEmoji.includes(key)).toBe(true);
+      });
+    });
+
+    it('should not contain invalid emoji', async () => {
+      await initEmojiMap();
+
+      const allEmoji = Object.keys(getAllEmoji());
+      Object.keys(invalidEmoji).forEach((key) => {
+        expect(allEmoji.includes(key)).toBe(false);
+      });
+    });
+
+    it('fixes broken pride emoji', async () => {
+      clearEmojiMock();
+      await initEmojiMock({
+        gay_pride_flag: {
+          c: 'flags',
+          // Without a zero-width joiner
+          e: '🏳🌈',
+          name: 'gay_pride_flag',
+          u: '6.0',
+        },
+      });
+
+      expect(getAllEmoji()).toEqual({
+        gay_pride_flag: {
+          c: 'flags',
+          // With a zero-width joiner
+          e: '🏳️‍🌈',
+          name: 'gay_pride_flag',
+          u: '6.0',
+        },
+      });
+    });
   });
 
   describe('glEmojiTag', () => {
@@ -378,32 +433,14 @@ describe('emoji', () => {
   });
 
   describe('searchEmoji', () => {
-    const emojiFixture = Object.keys(mockEmojiData).reduce((acc, k) => {
-      const { name, e, u, d } = mockEmojiData[k];
-      acc[k] = { name, e: sanitize(e), u, d };
-
-      return acc;
-    }, {});
-
     it.each([undefined, null, ''])("should return all emoji when the input is '%s'", (input) => {
       const search = searchEmoji(input);
 
-      const expected = [
-        'atom',
-        'bomb',
-        'construction_worker_tone5',
-        'five',
-        'grey_question',
-        'black_heart',
-        'heart',
-        'custard',
-        'star',
-        'xss',
-      ].map((name) => {
+      const expected = Object.keys(validEmoji).map((name) => {
         return {
-          emoji: emojiFixture[name],
+          emoji: mockEmojiData[name],
           field: 'd',
-          fieldValue: emojiFixture[name].d,
+          fieldValue: mockEmojiData[name].d,
           score: 0,
         };
       });
@@ -453,7 +490,7 @@ describe('emoji', () => {
         const { field, score, fieldValue, name } = item;
 
         return {
-          emoji: emojiFixture[name],
+          emoji: mockEmojiData[name],
           field,
           fieldValue,
           score,
@@ -564,9 +601,9 @@ describe('emoji', () => {
         const { field, score, name } = item;
 
         return {
-          emoji: emojiFixture[name],
+          emoji: mockEmojiData[name],
           field,
-          fieldValue: emojiFixture[name][field],
+          fieldValue: mockEmojiData[name][field],
           score,
         };
       });
@@ -622,13 +659,4 @@ describe('emoji', () => {
       expect(sortEmoji(scoredItems)).toEqual(expected);
     });
   });
-
-  describe('sanitize emojis', () => {
-    it('should return sanitized emoji', () => {
-      expect(getEmojiInfo('xss')).toEqual({
-        ...mockEmojiData.xss,
-        e: '<img src="x">',
-      });
-    });
-  });
 });

spec/frontend/gfm_auto_complete_spec.js

@@ -3,7 +3,7 @@ import MockAdapter from 'axios-mock-adapter';
 import $ from 'jquery';
 import labelsFixture from 'test_fixtures/autocomplete_sources/labels.json';
 import GfmAutoComplete, { membersBeforeSave, highlighter } from 'ee_else_ce/gfm_auto_complete';
-import { initEmojiMock } from 'helpers/emoji';
+import { initEmojiMock, clearEmojiMock } from 'helpers/emoji';
 import '~/lib/utils/jquery_at_who';
 import { TEST_HOST } from 'helpers/test_constants';
 import waitForPromises from 'helpers/wait_for_promises';
@@ -803,8 +803,6 @@ describe('GfmAutoComplete', () => {
   });
 
   describe('emoji', () => {
-    let mock;
-
     const mockItem = {
       'atwho-at': ':',
       emoji: {
@@ -818,14 +816,14 @@ describe('GfmAutoComplete', () => {
     };
 
     beforeEach(async () => {
-      mock = await initEmojiMock();
+      await initEmojiMock();
 
       await new GfmAutoComplete({}).loadEmojiData({ atwho() {}, trigger() {} }, ':');
       if (!GfmAutoComplete.glEmojiTag) throw new Error('emoji not loaded');
     });
 
     afterEach(() => {
-      mock.restore();
+      clearEmojiMock();
     });
 
     describe('Emoji.templateFunction', () => {

spec/frontend/set_status_modal/set_status_modal_wrapper_spec.js

 import { GlModal, GlFormCheckbox } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
-import { initEmojiMock } from 'helpers/emoji';
+import { initEmojiMock, clearEmojiMock } from 'helpers/emoji';
 import * as UserApi from '~/api/user_api';
 import EmojiPicker from '~/emoji/components/picker.vue';
 import createFlash from '~/flash';
@@ -12,7 +12,6 @@ jest.mock('~/flash');
 
 describe('SetStatusModalWrapper', () => {
   let wrapper;
-  let mockEmoji;
   const $toast = {
     show: jest.fn(),
   };
@@ -63,12 +62,12 @@ describe('SetStatusModalWrapper', () => {
 
   afterEach(() => {
     wrapper.destroy();
-    mockEmoji.restore();
+    clearEmojiMock();
   });
 
   describe('with minimum props', () => {
     beforeEach(async () => {
-      mockEmoji = await initEmojiMock();
+      await initEmojiMock();
       wrapper = createComponent();
       return initModal();
     });
@@ -112,7 +111,7 @@ describe('SetStatusModalWrapper', () => {
 
   describe('improvedEmojiPicker is true', () => {
     beforeEach(async () => {
-      mockEmoji = await initEmojiMock();
+      await initEmojiMock();
       wrapper = createComponent({}, true);
       return initModal();
     });
@@ -126,7 +125,7 @@ describe('SetStatusModalWrapper', () => {
 
   describe('with no currentMessage set', () => {
     beforeEach(async () => {
-      mockEmoji = await initEmojiMock();
+      await initEmojiMock();
       wrapper = createComponent({ currentMessage: '' });
       return initModal();
     });
@@ -146,7 +145,7 @@ describe('SetStatusModalWrapper', () => {
 
   describe('with no currentEmoji set', () => {
     beforeEach(async () => {
-      mockEmoji = await initEmojiMock();
+      await initEmojiMock();
       wrapper = createComponent({ currentEmoji: '' });
       return initModal();
     });
@@ -161,7 +160,7 @@ describe('SetStatusModalWrapper', () => {
 
     describe('with no currentMessage set', () => {
       beforeEach(async () => {
-        mockEmoji = await initEmojiMock();
+        await initEmojiMock();
         wrapper = createComponent({ currentEmoji: '', currentMessage: '' });
         return initModal();
       });
@@ -174,7 +173,7 @@ describe('SetStatusModalWrapper', () => {
 
   describe('with currentClearStatusAfter set', () => {
     beforeEach(async () => {
-      mockEmoji = await initEmojiMock();
+      await initEmojiMock();
       wrapper = createComponent({ currentClearStatusAfter: '2021-01-01 00:00:00 UTC' });
       return initModal();
     });
@@ -190,7 +189,7 @@ describe('SetStatusModalWrapper', () => {
   describe('update status', () => {
     describe('succeeds', () => {
       beforeEach(async () => {
-        mockEmoji = await initEmojiMock();
+        await initEmojiMock();
         wrapper = createComponent();
         await initModal();
 
@@ -246,7 +245,7 @@ describe('SetStatusModalWrapper', () => {
 
     describe('success message', () => {
       beforeEach(async () => {
-        mockEmoji = await initEmojiMock();
+        await initEmojiMock();
         wrapper = createComponent({ currentEmoji: '', currentMessage: '' });
         jest.spyOn(UserApi, 'updateUserStatus').mockResolvedValue();
         return initModal({ mockOnUpdateSuccess: false });
@@ -262,7 +261,7 @@ describe('SetStatusModalWrapper', () => {
 
     describe('with errors', () => {
       beforeEach(async () => {
-        mockEmoji = await initEmojiMock();
+        await initEmojiMock();
         wrapper = createComponent();
         await initModal();
 
@@ -279,7 +278,7 @@ describe('SetStatusModalWrapper', () => {
 
     describe('error message', () => {
       beforeEach(async () => {
-        mockEmoji = await initEmojiMock();
+        await initEmojiMock();
         wrapper = createComponent({ currentEmoji: '', currentMessage: '' });
         jest.spyOn(UserApi, 'updateUserStatus').mockRejectedValue();
         return initModal({ mockOnUpdateFailure: false });

yarn.lock

@@ -5041,7 +5041,12 @@ emittery@^0.7.1:
   resolved "https://registry.yarnpkg.com/emittery/-/emittery-0.7.1.tgz#c02375a927a40948c0345cc903072597f5270451"
   integrity sha512-d34LN4L6h18Bzz9xpoku2nPwKxCPlPMr3EEKTkoEBi+1/+b0lcRkRJ1UVyyZaKNeqGR3swcGl6s390DNO4YVgQ==
 
-emoji-regex@^7.0.1, emoji-regex@^7.0.3:
+emoji-regex@^10.0.0:
+  version "10.0.0"
+  resolved "https://registry.yarnpkg.com/emoji-regex/-/emoji-regex-10.0.0.tgz#96559e19f82231b436403e059571241d627c42b8"
+  integrity sha512-KmJa8l6uHi1HrBI34udwlzZY1jOEuID/ft4d8BSSEdRyap7PwBEt910453PJa5MuGvxkLqlt4Uvhu7tttFHViw==
+
+emoji-regex@^7.0.1:
   version "7.0.3"
   resolved "https://registry.yarnpkg.com/emoji-regex/-/emoji-regex-7.0.3.tgz#933a04052860c85e83c122479c4748a8e4c72156"
   integrity sha512-CwBLREIQ7LvYFB0WyRvwhq5N5qPhc6PMjD6bYggFlI5YyDgl+0vxq5VHbMOFqLg7hfWzmu8T5Z1QofhmTIhItA==