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 crash related to escaping html entites #5670

Merged
merged 1 commit into from
Dec 27, 2019

Conversation

pablode
Copy link
Contributor

@pablode pablode commented Dec 23, 2019

Purpose / Description

The Matcher.appendReplacement() method used when escaping html entities treats backslashes (\) and dollar signs ($) differently than the rest of the replacement string. Due to the input not being properly escaped, certain sequences (f.i. "&bsop;") lead to crashes.

Fixes

This fixes issue #5638.

Approach

We use the Matcher.quoteReplacement() helper method to escape the string.

How Has This Been Tested?

I've tested the fix multiple times using an Android 28 emulator.

Checklist

Please, go through these checks before submitting the PR.

  • You have not changed whitespace unnecessarily (it makes diffs hard to read)
  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • Your code follows the style of the project (e.g. never omit braces in if statements)
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code

This happened because Matcher.appendReplacement() treats backslashes (\) and dollar signs ($) differently than the rest of the replacement string. By using the utility method Matcher.quoteReplacement(), we can obtain a literal replacement string.
@mikehardy
Copy link
Member

@pablode champion! I'll look at that and try to get it to release quickly

@mikehardy mikehardy added 2.9.x Affects the 2.9 branch Priority-Critical Review High Priority Request for high priority review labels Dec 23, 2019
@mikehardy
Copy link
Member

mikehardy commented Dec 23, 2019

Once someone has one of these characters in their collection, it can't be edited to fix it which breaks reviewing that card or fixing it completely - thus the priority

Copy link
Member

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

seems like the right fix for the issue and uses an API that is available on all our supported SDK versions, thanks @pablode

@timrae could we get this queued for the next build as well (assuming it passes CI, I'm watching that now)

@mikehardy
Copy link
Member

@timrae passed CI, this should be all good to go

@timrae timrae merged commit 29ee9e4 into ankidroid:master Dec 27, 2019
@mikehardy mikehardy removed the Review High Priority Request for high priority review label Dec 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.9.x Affects the 2.9 branch Priority-Medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants