From e76a149e14c119eba79383c791ed17da5ec18d98 Mon Sep 17 00:00:00 2001 From: Danica Shen Date: Wed, 2 Oct 2024 22:43:35 +0100 Subject: [PATCH] fix(27428): fix if we type enter anything followed by a \ in settings search (#27432) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In JavaScript, backslashes are used as escape characters in strings and regular expressions, which can cause issues when an unescaped backslash is present in a user input. To handle this, we'll need to escape any special characters in the user input before constructing the regular expression. ## **Description** [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27432?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/27428, https://github.com/MetaMask/metamask-extension/issues/26945 ## **Manual testing steps** 1. Go to settings 2. type foo\ 3. press enter should not crash ## **Screenshots/Recordings** ### **Before** ### **After** https://github.com/user-attachments/assets/fb7336e9-7e41-4571-ae84-64c5ef5c3398 ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- ui/helpers/utils/settings-search.js | 15 +- ui/helpers/utils/settings-search.test.js | 189 +++++++++++++++++++++++ 2 files changed, 202 insertions(+), 2 deletions(-) diff --git a/ui/helpers/utils/settings-search.js b/ui/helpers/utils/settings-search.js index af13e1d71c65..07b4501c0208 100644 --- a/ui/helpers/utils/settings-search.js +++ b/ui/helpers/utils/settings-search.js @@ -68,7 +68,7 @@ export function handleSettingsRefs(t, tabMessage, settingsRefs) { } } -function colorText(menuElement, regex) { +export function colorText(menuElement, regex) { if (menuElement !== null) { let elemText = menuElement.innerHTML; elemText = elemText.replace('&', '&'); @@ -83,9 +83,20 @@ function colorText(menuElement, regex) { } } +/** + * Replaces any special characters in the input string that have a meaning in regular expressions + * (such as \, *, +, ?, etc.) with their escaped versions (e.g., \ becomes \\). + * + * @param input - The input string to be escaped for use in a regular expression. + * @returns The escaped string safe for use in a regular expression. + */ +export const escapeRegExp = (input) => { + return input.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // Escapes special characters +}; + export function highlightSearchedText() { const searchElem = document.getElementById('search-settings'); - const searchRegex = new RegExp(searchElem.value, 'gi'); + const searchRegex = new RegExp(escapeRegExp(searchElem.value), 'gi'); const results = document.querySelectorAll( '.settings-page__header__search__list__item', ); diff --git a/ui/helpers/utils/settings-search.test.js b/ui/helpers/utils/settings-search.test.js index 1e440b8f0aff..bb1637ec2cef 100644 --- a/ui/helpers/utils/settings-search.test.js +++ b/ui/helpers/utils/settings-search.test.js @@ -5,6 +5,9 @@ import { getNumberOfSettingRoutesInTab, handleSettingsRefs, getSpecificSettingsRoute, + escapeRegExp, + colorText, + highlightSearchedText, } from './settings-search'; const t = (key) => { @@ -199,6 +202,42 @@ describe('Settings Search Utils', () => { }); }); + describe('escapeRegExp', () => { + it('should escape special characters for use in regular expressions', () => { + const input = '.*+?^${}()|[]\\'; + const expectedOutput = '\\.\\*\\+\\?\\^\\$\\{\\}\\(\\)\\|\\[\\]\\\\'; + expect(escapeRegExp(input)).toBe(expectedOutput); + }); + + it('should return the same string if no special characters are present', () => { + const input = 'hello'; + expect(escapeRegExp(input)).toBe(input); + }); + + it('should escape only the special characters in a mixed string', () => { + const input = 'hello.*world'; + const expectedOutput = 'hello\\.\\*world'; + expect(escapeRegExp(input)).toBe(expectedOutput); + }); + + it('should handle an empty string correctly', () => { + const input = ''; + expect(escapeRegExp(input)).toBe(''); + }); + + it('should escape backslashes properly', () => { + const input = '\\'; + const expectedOutput = '\\\\'; + expect(escapeRegExp(input)).toBe(expectedOutput); + }); + + it('should escape backslashes with content properly', () => { + const input = 'foobar\\'; + const expectedOutput = 'foobar\\\\'; + expect(escapeRegExp(input)).toBe(expectedOutput); + }); + }); + // Can't be tested without DOM element describe('handleSettingsRefs', () => { it('should handle general refs', () => { @@ -223,4 +262,154 @@ describe('Settings Search Utils', () => { ); }); }); + + describe('colorText', () => { + let mockElement; + + beforeEach(() => { + mockElement = { + innerHTML: 'Test & string with multiple words', + }; + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should highlight text that matches the regex', () => { + const regex = /string/giu; + colorText(mockElement, regex); + expect(mockElement.innerHTML).toBe( + 'Test & string with multiple words', + ); + }); + + it('should correctly decode & to &', () => { + const regex = /&/giu; + colorText(mockElement, regex); + expect(mockElement.innerHTML).toBe( + 'Test & string with multiple words', + ); + }); + + it('should remove any existing highlight spans before applying new highlights', () => { + mockElement.innerHTML = + 'Test & string with multiple words'; + const regex = /multiple/giu; + colorText(mockElement, regex); + expect(mockElement.innerHTML).toBe( + 'Test & string with multiple words', + ); + }); + + it('should not modify innerHTML if menuElement is null', () => { + const regex = /string/giu; + const nullElement = null; + colorText(nullElement, regex); + expect(nullElement).toBeNull(); + }); + + it('should not highlight anything if regex doesn’t match', () => { + const regex = /nomatch/giu; + colorText(mockElement, regex); + expect(mockElement.innerHTML).toBe('Test & string with multiple words'); + }); + }); + + describe('highlightSearchedText', () => { + let searchElem; + let mockResultItems; + let mockMenuTabElement; + let mockMenuSectionElement; + + beforeEach(() => { + searchElem = document.createElement('input'); + searchElem.id = 'search-settings'; + searchElem.value = 'test'; + document.body.appendChild(searchElem); + + // Mock result list items + mockResultItems = [...Array(2)].map(() => { + const item = document.createElement('div'); + item.classList.add('settings-page__header__search__list__item'); + + mockMenuTabElement = document.createElement('div'); + mockMenuTabElement.classList.add( + 'settings-page__header__search__list__item__tab', + ); + mockMenuTabElement.innerHTML = 'Test tab'; + + mockMenuSectionElement = document.createElement('div'); + mockMenuSectionElement.classList.add( + 'settings-page__header__search__list__item__section', + ); + mockMenuSectionElement.innerHTML = 'Test section'; + + item.appendChild(mockMenuTabElement); + item.appendChild(mockMenuSectionElement); + + return item; + }); + + mockResultItems.forEach((item) => document.body.appendChild(item)); + }); + + afterEach(() => { + document.body.innerHTML = ''; + }); + + it('should highlight the matching text in both menuTabElement and menuSectionElement', () => { + highlightSearchedText(); + mockResultItems.forEach((item) => { + const tabElement = item.querySelector( + '.settings-page__header__search__list__item__tab', + ); + const sectionElement = item.querySelector( + '.settings-page__header__search__list__item__section', + ); + expect(tabElement.innerHTML).toBe( + 'Test tab', + ); + expect(sectionElement.innerHTML).toBe( + 'Test section', + ); + }); + }); + + it('should not alter the innerHTML if no match is found', () => { + searchElem.value = 'nomatch'; + highlightSearchedText(); + mockResultItems.forEach((item) => { + const tabElement = item.querySelector( + '.settings-page__header__search__list__item__tab', + ); + const sectionElement = item.querySelector( + '.settings-page__header__search__list__item__section', + ); + + expect(tabElement.innerHTML).toBe('Test tab'); + expect(sectionElement.innerHTML).toBe('Test section'); + }); + }); + + it('should do nothing if the search input is empty', () => { + searchElem.value = ''; + highlightSearchedText(); + mockResultItems.forEach((item) => { + const tabElement = item.querySelector( + '.settings-page__header__search__list__item__tab', + ); + const sectionElement = item.querySelector( + '.settings-page__header__search__list__item__section', + ); + + expect(tabElement.innerHTML).toBe( + 'Test tab', + ); + expect(sectionElement.innerHTML).toBe( + 'Test section', + ); + }); + }); + }); });