Skip to content

Commit

Permalink
fix(27428): fix if we type enter anything followed by a \ in settings…
Browse files Browse the repository at this point in the history
… search (#27432)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

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**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27432?quickstart=1)

## **Related issues**

Fixes: #27428,
#26945

## **Manual testing steps**

1. Go to settings
2. type foo\
3. press enter should not crash

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**


https://github.com/user-attachments/assets/fb7336e9-7e41-4571-ae84-64c5ef5c3398


<!-- [screenshots/recordings] -->

## **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.
  • Loading branch information
DDDDDanica authored Oct 2, 2024
1 parent 32939f7 commit e76a149
Show file tree
Hide file tree
Showing 2 changed files with 202 additions and 2 deletions.
15 changes: 13 additions & 2 deletions ui/helpers/utils/settings-search.js
Original file line number Diff line number Diff line change
Expand Up @@ -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('&amp;', '&');
Expand All @@ -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',
);
Expand Down
189 changes: 189 additions & 0 deletions ui/helpers/utils/settings-search.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import {
getNumberOfSettingRoutesInTab,
handleSettingsRefs,
getSpecificSettingsRoute,
escapeRegExp,
colorText,
highlightSearchedText,
} from './settings-search';

const t = (key) => {
Expand Down Expand Up @@ -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', () => {
Expand All @@ -223,4 +262,154 @@ describe('Settings Search Utils', () => {
);
});
});

describe('colorText', () => {
let mockElement;

beforeEach(() => {
mockElement = {
innerHTML: 'Test &amp; 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 & <span class="settings-page__header__search__list__item__highlight">string</span> with multiple words',
);
});

it('should correctly decode &amp; to &', () => {
const regex = /&/giu;
colorText(mockElement, regex);
expect(mockElement.innerHTML).toBe(
'Test <span class="settings-page__header__search__list__item__highlight">&</span> string with multiple words',
);
});

it('should remove any existing highlight spans before applying new highlights', () => {
mockElement.innerHTML =
'Test &amp; <span class="settings-page__header__search__list__item__highlight">string</span> with multiple words';
const regex = /multiple/giu;
colorText(mockElement, regex);
expect(mockElement.innerHTML).toBe(
'Test & string with <span class="settings-page__header__search__list__item__highlight">multiple</span> 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(
'<span class="settings-page__header__search__list__item__highlight">Test</span> tab',
);
expect(sectionElement.innerHTML).toBe(
'<span class="settings-page__header__search__list__item__highlight">Test</span> 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(
'<span class="settings-page__header__search__list__item__highlight"></span>T<span class="settings-page__header__search__list__item__highlight"></span>e<span class="settings-page__header__search__list__item__highlight"></span>s<span class="settings-page__header__search__list__item__highlight"></span>t<span class="settings-page__header__search__list__item__highlight"></span> <span class="settings-page__header__search__list__item__highlight"></span>t<span class="settings-page__header__search__list__item__highlight"></span>a<span class="settings-page__header__search__list__item__highlight"></span>b<span class="settings-page__header__search__list__item__highlight"></span>',
);
expect(sectionElement.innerHTML).toBe(
'<span class="settings-page__header__search__list__item__highlight"></span>T<span class="settings-page__header__search__list__item__highlight"></span>e<span class="settings-page__header__search__list__item__highlight"></span>s<span class="settings-page__header__search__list__item__highlight"></span>t<span class="settings-page__header__search__list__item__highlight"></span> <span class="settings-page__header__search__list__item__highlight"></span>s<span class="settings-page__header__search__list__item__highlight"></span>e<span class="settings-page__header__search__list__item__highlight"></span>c<span class="settings-page__header__search__list__item__highlight"></span>t<span class="settings-page__header__search__list__item__highlight"></span>i<span class="settings-page__header__search__list__item__highlight"></span>o<span class="settings-page__header__search__list__item__highlight"></span>n<span class="settings-page__header__search__list__item__highlight"></span>',
);
});
});
});
});

0 comments on commit e76a149

Please sign in to comment.