Skip to content

fix: display of reverse string in signature request message#23751

Merged
jpuri merged 4 commits intodevelopfrom
reverse_string_fix
Mar 27, 2024
Merged

fix: display of reverse string in signature request message#23751
jpuri merged 4 commits intodevelopfrom
reverse_string_fix

Conversation

@jpuri
Copy link
Copy Markdown
Contributor

@jpuri jpuri commented Mar 27, 2024

Description

Fix display of reverse string in signature request message.

Related issues

Fixes: #23691

Manual testing steps

For reproducing, open the console and paste this (changing the address to your connected address)

await window.ethereum.request({"method":"personal_sign","params":["0x07Be9763a718C0539017E2Ab6fC42853b4aEeb6B", "Sign into \u202E EVIL"],"id":1})

Screenshots/Recordings

Screenshot 2024-03-27 at 11 54 17 AM

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • 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 format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). 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.

@jpuri jpuri added team-confirmations-secure-ux-PR PRs from the confirmations team needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) confirmation-redesign labels Mar 27, 2024
@jpuri jpuri requested review from a team as code owners March 27, 2024 06:27
@github-actions
Copy link
Copy Markdown
Contributor

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.

<ConfirmInfoRowAddress address={value} />
) : (
<ConfirmInfoRowText text={value} />
<ConfirmInfoRowText text={sanitizeString(value)} />
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to do some fixes in this component to be able to unit test cover it. It will be in a followup PR for this issue: https://app.zenhub.com/workspaces/-confirmations-secure-ux-6245e6e2348677001213b8d2/issues/gh/metamask/metamask-extension/23689

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [951898b]
Page Load Metrics (730 ± 463 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint741991213216
domContentLoaded106435178
load712507730963463
domInteractive106435178
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [78749e2]
Page Load Metrics (950 ± 513 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint712771265727
domContentLoaded1078282010
load5824399501068513
domInteractive1078282010
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 48 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@jpuri jpuri merged commit 1eeaf82 into develop Mar 27, 2024
@jpuri jpuri deleted the reverse_string_fix branch March 27, 2024 14:36
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2024
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Mar 27, 2024
@metamaskbot metamaskbot added the release-11.15.0 Issue or pull request that will be included in release 11.15.0 label Mar 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

confirmation-redesign release-11.15.0 Issue or pull request that will be included in release 11.15.0 team-confirmations-secure-ux-PR PRs from the confirmations team

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[Bug]: Confirmations Re-design - Reverse String not properly Handled

4 participants