Skip to content

reject RBNF plural rules whose divisor overflows to zero#4004

Open
rootvector2 wants to merge 2 commits into
unicode-org:mainfrom
rootvector2:rbnf-zero-divisor
Open

reject RBNF plural rules whose divisor overflows to zero#4004
rootvector2 wants to merge 2 commits into
unicode-org:mainfrom
rootvector2:rbnf-zero-divisor

Conversation

@rootvector2

@rootvector2 rootvector2 commented May 27, 2026

Copy link
Copy Markdown

A slash divisor descriptor with a $(cardinal,...)$ body sets rulePatternFormat but creates no modulus or integral-part substitution, so the divisor checks in those substitution constructors never run. When radix^exponent overflows 64 bits the power helper wraps to 0 (util64_pow in ICU4C, NFRule.power in ICU4J), and doFormat() then divides the formatted number by that zero divisor: integer divide by zero, SIGFPE on x86-64 in C, ArithmeticException: / by zero in Java.

extractSubstitutions now rejects the rule as soon as the plural format is built and getDivisor() == 0, mirroring the existing zero-divisor guards in the modulus and integral-part substitutions. ICU4C surfaces U_PARSE_ERROR; ICU4J throws IllegalStateException, matching how those substitutions already reject a zero divisor.

Confirmed the same crash exists in ICU4J (the Number Format Tester / numbers.jsp throws / by zero), so the fix and the test are mirrored to both. The C++ TestDividedByZero and the new Java TestDivisorOverflow both fail without the fix and pass with it.

Checklist

  • Required: Issue filed: ICU-NNNNN
  • Required: The PR title must be prefixed with a JIRA Issue number. Example: "ICU-NNNNN Fix xyz"
  • Required: Each commit message must be prefixed with a JIRA Issue number. Example: "ICU-NNNNN Fix xyz"
  • Issue accepted (done by Technical Committee after discussion)
  • Tests included, if applicable
  • API docs and/or User Guide docs changed or added, if applicable
  • Approver: Feel free to merge on my behalf

A slash divisor descriptor with a $(cardinal,...)$ body sets rulePatternFormat but creates no modulus or integral-part substitution, so the divisor checks in those substitutions never run. When radix to the power exponent overflows 64 bits util64_pow returns 0 and NFRule::doFormat divides by that zero divisor. extractSubstitutions now rejects the rule with U_PARSE_ERROR.
@markusicu

Copy link
Copy Markdown
Member
  • please restore and fill out the pull request template
  • please create a Jira ticket with the problem description
  • @grhoten @richgillam FYI

@markusicu

Copy link
Copy Markdown
Member

Please also check if this is a problem in Java -- ideally by porting the unit test.

@grhoten grhoten left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I confirmed that the Number Format Tester also throws a "/ by zero" exception, which uses ICU4J. So the change needs to be mirrored to both.

These changes are just modifying how the failure is surfaced. I find it unlikely that this issue would have surfaced through typical usage of RBNF in any language. I think only some French variants use the divisor like this, and it's only for numbers 60-99. While it's a valid issue, it's unlikely to be encountered outside of synthetic rule making with large numbers.

Please follow the directions provided by Markus to get this change mergeable.

@rootvector2

Copy link
Copy Markdown
Author

Confirmed it's the same bug in ICU4J: NFRule.power wraps to 0 on that overflow and doFormat divides by it, which is the / by zero from the Number Format Tester. Mirrored the guard in NFRule.extractSubstitutions (throws IllegalStateException, like the substitution zero-divisor checks already do) and ported the test as RbnfTest.TestDivisorOverflow. It fails without the fix and passes with it.

Agree this only changes how the failure surfaces, and you'd really only hit it with synthetic large-divisor rules rather than real RBNF data. Rejecting at construction just matches the existing zero-divisor guards instead of dividing by zero at format time.

Restored and filled out the PR template. On the Jira ticket: I'm not on the team so I can't file in the tracker directly. I can submit it through the bug page, or if it's easier for you to open one I'll prefix the title and commits with the number.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants