metamask-mobile icon indicating copy to clipboard operation
metamask-mobile copied to clipboard

fix: Address issues with inaccurate regex expressions

Open NicholasEllul opened this issue 1 year ago • 1 comments

Description

This PR addresses a few issues related to how we were using regex expressions. In particular, this helps address some common mistakes often made when using URLs in a regex statement.

Outline of changes

f203e3774613f6a3ec1650be157ceda5d40acd2c

This commit escapes cases where we were wanting to use the portfolio URL in a regex statement, and also splits PORTFOLIO.URL and PORTFOLIO.URL_REGEX into two distinct constants in app constants. This better reduces the risk of using the URL incorrectly in the future.

Previously, this regex would incorrectly match when visiting websites such as https://portfolioxmetamask.io due to the period being inferred as a wildcard.

d183e65d5e290384600df33d9bfaecb8a8456c2f

This commit addresses a similar issue to that described above, except related to the bridging functionality of portfolio.

09f227e23ed9b23c5ef792d18df6a1f24d97169d

This commit fixes an issue where we were only escaping the first . found in a URL when using the inpage blocklist. Previously we had entries such as ani.gamer.com.tw; however, since only the first period was being escaped, we were unintentionally also blocking things such as ani.gamerxcom.tw etc.

This commit also fixes the fact that we were missing a leading anchor ^ in the regex expression. This means that we would block the domain if the matched string occurred anywhere in the URL. For an example, https://google.com?search=uscourts.gov would be a blocked domain since it ended in uscourts.gov. Adding the leading anchor addresses this so we only match the correct domain.

Related issues

https://github.com/MetaMask/mobile-planning/issues/1571

Pre-merge author checklist

  • [x] I’ve followed MetaMask Coding Standards.
  • [x] I've clearly explained what problem this PR is solving and how it is solved.
  • [x] I've linked related issues
  • [ ] I've included manual testing steps
  • [x] I've included screenshots/recordings if applicable
  • [x] I’ve included tests if applicable
  • [x] I’ve documented my code using JSDoc format if applicable
  • [X] I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
  • [X] I’ve properly set the pull request status:
    • [X] In case it's not yet "ready for review", I've set it to "draft".
    • [X] In case it's "ready for review", I've changed it from "draft" to "non-draft".

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.

NicholasEllul avatar Feb 21 '24 20:02 NicholasEllul

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

github-actions[bot] avatar Feb 21 '24 20:02 github-actions[bot]

Succeeded by #8675 and #8674

NicholasEllul avatar Feb 22 '24 16:02 NicholasEllul