reject RBNF plural rules whose divisor overflows to zero#4004
reject RBNF plural rules whose divisor overflows to zero#4004rootvector2 wants to merge 2 commits into
Conversation
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.
|
|
Please also check if this is a problem in Java -- ideally by porting the unit test. |
grhoten
left a comment
There was a problem hiding this comment.
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.
|
Confirmed it's the same bug in ICU4J: 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. |
A slash divisor descriptor with a
$(cardinal,...)$body setsrulePatternFormatbut creates no modulus or integral-part substitution, so the divisor checks in those substitution constructors never run. Whenradix^exponentoverflows 64 bits the power helper wraps to 0 (util64_powin ICU4C,NFRule.powerin ICU4J), anddoFormat()then divides the formatted number by that zero divisor: integer divide by zero, SIGFPE on x86-64 in C,ArithmeticException: / by zeroin Java.extractSubstitutionsnow rejects the rule as soon as the plural format is built andgetDivisor() == 0, mirroring the existing zero-divisor guards in the modulus and integral-part substitutions. ICU4C surfacesU_PARSE_ERROR; ICU4J throwsIllegalStateException, matching how those substitutions already reject a zero divisor.Confirmed the same crash exists in ICU4J (the Number Format Tester /
numbers.jspthrows/ by zero), so the fix and the test are mirrored to both. The C++TestDividedByZeroand the new JavaTestDivisorOverflowboth fail without the fix and pass with it.Checklist