Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Backup code download with quotes in translations #494

Merged
merged 11 commits into from
Nov 28, 2022

Conversation

dd32
Copy link
Member

@dd32 dd32 commented Nov 9, 2022

Fixes #460

  • encodeURI() doesn't encode enough characters for non-english languages.
  • encodeURIComponent() doesn't encode enough characters for usage within data: uri's
  • i18n.title should not be HTML escaped if it's not going to be used within a HTML context. This results in the translation string having HTML entities. An alternative would be to decode HTML entities client-side in JS and then use it that value, but encoding it in the first place here provides no security benefits.

…is is needed in the event response.data.i18n.title contains a quote or other similar character.
…t this to be HTML encoded as it's presented within a plaintext file.
@dd32 dd32 changed the title Download Backup file with quotes in translations Fix Backup code download with quotes in translations Nov 9, 2022
@jeffpaul jeffpaul added this to the 0.8.0 milestone Nov 9, 2022
@jeffpaul
Copy link
Member

jeffpaul commented Nov 9, 2022

Open to review from @iandunn @georgestephanis or @kasparsd to help land this in the 0.8.0 release

Copy link
Collaborator

@kasparsd kasparsd left a comment

Choose a reason for hiding this comment

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

Looks good!

providers/class-two-factor-backup-codes.php Outdated Show resolved Hide resolved
providers/class-two-factor-backup-codes.php Outdated Show resolved Hide resolved
@dd32 dd32 force-pushed the fix/460-translation-with-quote branch from f770e6e to 7c0a548 Compare November 11, 2022 06:32
…will delete the code.

This shouldn't have affected the test, as `$current_user` and `$user` should not match, but for safety..
@dd32 dd32 merged commit 555d886 into WordPress:master Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Download backup code doesn't generate working download link in french
3 participants