Bug 234817 - [JSC] Fix Intl.PluralRules.selectRange input validation
Summary: [JSC] Fix Intl.PluralRules.selectRange input validation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-01-03 06:45 PST by Yusuke Suzuki
Modified: 2022-01-03 09:48 PST (History)
8 users (show)

See Also:


Attachments
Patch (5.73 KB, patch)
2022-01-03 06:45 PST, Yusuke Suzuki
ashvayka: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (5.74 KB, patch)
2022-01-03 09:17 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2022-01-03 06:45:17 PST
[JSC] Fix Intl.PluralRules.selectRange input validation
Comment 1 Yusuke Suzuki 2022-01-03 06:45:56 PST
Created attachment 448230 [details]
Patch
Comment 2 Alexey Shvayka 2022-01-03 08:26:30 PST
Comment on attachment 448230 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=448230&action=review

> Source/JavaScriptCore/ChangeLog:8
> +        Add specified argument validation to Intl.PluralRules.selectRange.

Should we include a spec link (https://tc39.es/proposal-intl-numberformat-v3/out/pluralrules/proposed.html#sec-intl.pluralrules.prototype.selectrange)? It's a bit non-trivial to find.

> Source/JavaScriptCore/runtime/IntlPluralRules.cpp:288
> +    if (isNegativeZero(end) && start >= 0)

Hmm, this is interesting: spec's numeric operators (< / <= / > / >=) for Number should "refer to the relevant operations within IEEE 754-2019" (paragraph 3 of https://tc39.es/ecma262/multipage/notational-conventions.html#sec-mathematical-operations).
Does the IEEE 754 differentiate between -0 and 0 in comparison operators? Comparing `-0.0 < 0.0` C++ doubles yields `false`.
Comment 3 Yusuke Suzuki 2022-01-03 09:11:38 PST
Comment on attachment 448230 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=448230&action=review

>> Source/JavaScriptCore/ChangeLog:8
>> +        Add specified argument validation to Intl.PluralRules.selectRange.
> 
> Should we include a spec link (https://tc39.es/proposal-intl-numberformat-v3/out/pluralrules/proposed.html#sec-intl.pluralrules.prototype.selectrange)? It's a bit non-trivial to find.

Nice, added.

>> Source/JavaScriptCore/runtime/IntlPluralRules.cpp:288
>> +    if (isNegativeZero(end) && start >= 0)
> 
> Hmm, this is interesting: spec's numeric operators (< / <= / > / >=) for Number should "refer to the relevant operations within IEEE 754-2019" (paragraph 3 of https://tc39.es/ecma262/multipage/notational-conventions.html#sec-mathematical-operations).
> Does the IEEE 754 differentiate between -0 and 0 in comparison operators? Comparing `-0.0 < 0.0` C++ doubles yields `false`.

This is aligned to Intl.NumberFormat.formatRange implementation, and IIRC, there is a test for this in test262.
And ICU can distinguish -0 and +0, so I think we should check that here for the sane semantics.
Comment 4 Yusuke Suzuki 2022-01-03 09:17:44 PST
Created attachment 448247 [details]
Patch
Comment 5 EWS 2022-01-03 09:47:24 PST
Committed r287546 (245681@main): <https://commits.webkit.org/245681@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 448247 [details].
Comment 6 Radar WebKit Bug Importer 2022-01-03 09:48:19 PST
<rdar://problem/87068803>