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

ICU-22890 Add test to show lone surrogate cause infinity loop #3166

Conversation

FrankYFTang
Copy link
Contributor

Checklist
  • Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-22890
  • Required: The PR title must be prefixed with a JIRA Issue number.
  • Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
  • Required: Each commit message must be prefixed with a JIRA Issue number.
  • Issue accepted (done by Technical Committee after discussion)
  • Tests included, if applicable
  • API docs and/or User Guide docs changed or added, if applicable

@FrankYFTang
Copy link
Contributor Author

@catamorphism and @echeran This PR show the the problem

@catamorphism
Copy link
Contributor

Here's the fix:

$ git diff
diff --git a/icu4c/source/i18n/messageformat2_parser.cpp b/icu4c/source/i18n/messageformat2_parser.cpp
index fe5bfb48480..3d5ea6e560d 100644
--- a/icu4c/source/i18n/messageformat2_parser.cpp
+++ b/icu4c/source/i18n/messageformat2_parser.cpp
@@ -1785,7 +1785,7 @@ UnicodeString parser::parseText(UErrorCode& status) {
         return str;
     }
 
-    if (!(isTextChar(source[index] || source[index] == BACKSLASH))) {
+    if (!(isTextChar(source[index]) || source[index] == BACKSLASH)) {
         // Error -- text is expected here
         ERROR(parseError, status, index);
         return str;

The parentheses in the error check in parseText() were misplaced and the error condition was always false.

Note that #3092 fixes wide character parsing in general, but the above change fixes the infinite loop.

@catamorphism
Copy link
Contributor

Unfortunately, applying the fix exposes other parser bugs, which I think are fixed by #3063, which hasn't landed yet. I'll see if I can create a more minimal change set that will fix the tests that fail here.

@catamorphism
Copy link
Contributor

OK, see #3167 for a complete fix. Feel free to cherry-pick the extra patch out of there and add it to this PR, or close this PR in favor of that one, whichever you prefer.

@FrankYFTang
Copy link
Contributor Author

close this PR in favor of that one, whichever you prefer.

@FrankYFTang FrankYFTang deleted the ICU-22890-messageformatinfinity branch September 16, 2024 17:38
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.

2 participants