Skip to content

Commit 905030c

Browse files
authored
fix: catch when url parse throws (#15161)
<!-- 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. --> ## **Description** This PR aims to catch when the URL throws. <!-- 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? --> ## **Related issues** Fixes: #15008 ## **Manual testing steps** 1. Go to the test dapp 2. Click `Sign In With Ethereum (Bad Domain)` ## **Screenshots/Recordings** [siwe.webm](https://github.com/user-attachments/assets/1c4a2824-8e56-4087-8be1-01c82995d421) <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [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. ## **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.
1 parent cea89c6 commit 905030c

File tree

2 files changed

+68
-3
lines changed

2 files changed

+68
-3
lines changed

app/components/Views/confirmations/hooks/alerts/useDomainMismatchAlerts.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,54 @@ describe('useDomainMismatchAlerts', () => {
9797

9898
expect(result.current).toEqual([ALERT_MOCK]);
9999
});
100+
101+
it('falls back to requestData.origin when new URL(meta?.url) throws an error', () => {
102+
(isSIWESignatureRequest as jest.Mock).mockReturnValue(true);
103+
const ORIGIN_MOCK = 'https://metamask.github.io';
104+
const META_URL_MOCK = 'invalid-url';
105+
106+
const consoleWarnSpy = jest.spyOn(console, 'warn');
107+
108+
const { result } = renderHookWithProvider(() => useDomainMismatchAlerts(), {
109+
state: {
110+
...siweSignatureConfirmationState,
111+
engine: {
112+
...siweSignatureConfirmationState.engine,
113+
backgroundState: {
114+
...siweSignatureConfirmationState.engine.backgroundState,
115+
ApprovalController: {
116+
...siweSignatureConfirmationState.engine.backgroundState.ApprovalController,
117+
pendingApprovals: {
118+
...siweSignatureConfirmationState.engine.backgroundState.ApprovalController.pendingApprovals,
119+
'72424261-e22f-11ef-8e59-bf627a5d8354': {
120+
...siweSignatureConfirmationState.engine.backgroundState.ApprovalController.pendingApprovals['72424261-e22f-11ef-8e59-bf627a5d8354'],
121+
origin: ORIGIN_MOCK,
122+
requestData: {
123+
...siweSignatureConfirmationState.engine.backgroundState.ApprovalController.pendingApprovals['72424261-e22f-11ef-8e59-bf627a5d8354'].requestData,
124+
origin: ORIGIN_MOCK,
125+
meta: {
126+
url: META_URL_MOCK,
127+
},
128+
},
129+
},
130+
},
131+
},
132+
},
133+
},
134+
},
135+
});
136+
137+
expect(consoleWarnSpy).toHaveBeenCalledWith(
138+
'useDomainMismatchAlerts: error while parsing URL',
139+
expect.objectContaining({
140+
error: expect.any(Object),
141+
origin: ORIGIN_MOCK,
142+
metaUrl: META_URL_MOCK,
143+
}),
144+
);
145+
146+
expect(result.current).toEqual([ALERT_MOCK]);
147+
148+
consoleWarnSpy.mockRestore();
149+
});
100150
});

app/components/Views/confirmations/hooks/alerts/useDomainMismatchAlerts.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,29 @@ export default function useDomainMismatchAlerts(): Alert[] {
1414
const signatureRequest = useSignatureRequest();
1515

1616
const { requestData } = approvalRequest || {};
17+
const { meta } = requestData;
1718
const isSIWE = isSIWESignatureRequest(signatureRequest);
1819

19-
const { meta } = requestData;
20+
let originWithProtocol: string | undefined;
21+
22+
try {
23+
originWithProtocol =
24+
regex.urlHttpToHttps.test(requestData.origin) || !isSIWE
25+
? requestData.origin
26+
: new URL(meta?.url).origin;
27+
} catch (error) {
28+
console.warn('useDomainMismatchAlerts: error while parsing URL', {
29+
error,
30+
origin: requestData.origin,
31+
metaUrl: meta?.url,
32+
});
2033

21-
const originWithProtocol = regex.urlHttpToHttps.test(requestData.origin) || !isSIWE ? requestData.origin : new URL(meta?.url).origin;
34+
originWithProtocol = requestData.origin;
35+
}
2236

2337
const isInvalidSIWEDomain =
24-
isSIWE && !isValidSIWEOrigin({ ...requestData, origin: originWithProtocol } as WrappedSIWERequest);
38+
isSIWE &&
39+
!isValidSIWEOrigin({ ...requestData, origin: originWithProtocol } as WrappedSIWERequest);
2540

2641
const alerts = useMemo(() => {
2742
if (!isInvalidSIWEDomain) {

0 commit comments

Comments
 (0)