From f5c63e965e7bcabd6232d4bd7665ac14934b058a Mon Sep 17 00:00:00 2001 From: salimtb Date: Fri, 8 Mar 2024 19:17:25 +0100 Subject: [PATCH] fix: fix url bug display (#8877) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** There is a bug with the RPC url not being displayed correctly when trying to add a network via a dapp. This raises a security concern because the user can potentially add a malicious network if a network RPC URL is not shown. Furthermore, the height of the network added sheet extends further than that of prod. ## **Related issues** Fixes: [#1586 ](https://github.com/MetaMask/mobile-planning/issues/1586) ## **Manual testing steps** 1. Given I am on the browser view 2. And I connect my wallet to chainlist.wtf 3. When I add "Avalanche" 4. Then the add network sheet is displayed 5. But the RPC URL is not displayed correctly ## **Screenshots/Recordings** ### **Before** before-bug ### **After** fix-bug ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained what problem this PR is solving and how it is solved. - [x] I've linked related issues - [x] 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](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [x] I’ve properly set the pull request status: - [ ] 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. --- .../__snapshots__/index.test.tsx.snap | 2 +- .../NetworkVerificationInfo.test.tsx.snap | 2 +- .../NetworkSwitcher.test.tsx.snap | 2 +- app/util/hideKeyFromUrl.test.ts | 12 ++++++++++++ app/util/hideKeyFromUrl.ts | 19 +++++++++++++++++-- 5 files changed, 32 insertions(+), 5 deletions(-) diff --git a/app/components/UI/NetworkModal/__snapshots__/index.test.tsx.snap b/app/components/UI/NetworkModal/__snapshots__/index.test.tsx.snap index c6973deb960..604755ff9fe 100644 --- a/app/components/UI/NetworkModal/__snapshots__/index.test.tsx.snap +++ b/app/components/UI/NetworkModal/__snapshots__/index.test.tsx.snap @@ -425,7 +425,7 @@ exports[`NetworkDetails renders correctly 1`] = ` } } > - https:/ + https://localhost:8545 - http:/ + http://test.com - https:/ + https://evm.cronos.org { const sanitizedUrl = hideKeyFromUrl(urlString); expect(sanitizedUrl).toEqual(undefined); }); + + it('should not hide key from url', () => { + const urlString = 'https://www.example.com'; + const sanitizedUrl = hideKeyFromUrl(urlString); + expect(sanitizedUrl).toEqual('https://www.example.com'); + }); + + it('should hide key from url if protocol is not defined', () => { + const urlString = 'www.example.com/v1/1234'; + const sanitizedUrl = hideKeyFromUrl(urlString); + expect(sanitizedUrl).toEqual('www.example.com/v1'); + }); }); diff --git a/app/util/hideKeyFromUrl.ts b/app/util/hideKeyFromUrl.ts index 7a196758986..29a4c915d68 100644 --- a/app/util/hideKeyFromUrl.ts +++ b/app/util/hideKeyFromUrl.ts @@ -1,4 +1,19 @@ -const hideKeyFromUrl = (url: string | undefined) => - url?.substring(0, url.lastIndexOf('/')); +const hideKeyFromUrl = (url: string | undefined) => { + if (!url) return url; + + const regex = /^(https?:\/\/)(.*)$/; + const match = url.match(regex); + + if (match) { + const protocol = match[1]; + let restOfUrl = match[2]; + + // eslint-disable-next-line no-useless-escape + restOfUrl = restOfUrl.replace(/\/[^\/]*$/, ''); + return protocol + restOfUrl; + } + + return url?.substring(0, url.lastIndexOf('/')); +}; export default hideKeyFromUrl;