Skip to content

Commit 8e6e10f

Browse files
cscottGirgias
authored andcommitted
Fix #76093: Format strings w/o loss of precision w/ FORMAT_TYPE_DECIMAL
Passing the argument to NumberFormat::format() as a number loses precision if the value can not be represented precisely as a double or long integer. The icu library provides a "decimal number" type that avoids the loss of prevision when the value is passed as a string. Add a new FORMAT_TYPE_DECIMAL to explicitly request the argument be converted to a string and then passed to icu that way. Co-authored-by: Gina Peter Banyard <girgias@php.net>
1 parent 85225a8 commit 8e6e10f

8 files changed

+156
-12
lines changed

ext/intl/formatter/formatter.stub.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,8 @@ class NumberFormatter
171171
public const int TYPE_INT64 = UNKNOWN;
172172
/** @cvalue FORMAT_TYPE_DOUBLE */
173173
public const int TYPE_DOUBLE = UNKNOWN;
174+
/** @cvalue FORMAT_TYPE_DECIMAL */
175+
public const int TYPE_DECIMAL = UNKNOWN;
174176
/**
175177
* @deprecated
176178
* @cvalue FORMAT_TYPE_CURRENCY
@@ -189,7 +191,7 @@ public static function create(string $locale, int $style, ?string $pattern = nul
189191
* @tentative-return-type
190192
* @alias numfmt_format
191193
*/
192-
public function format(int|float $num, int $type = NumberFormatter::TYPE_DEFAULT): string|false {}
194+
public function format(int|float|string $num, int $type = NumberFormatter::TYPE_DEFAULT): string|false {}
193195

194196
/**
195197
* @param int $offset

ext/intl/formatter/formatter_arginfo.h

Lines changed: 8 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

ext/intl/formatter/formatter_format.c

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ PHP_FUNCTION( numfmt_format )
3535
FORMATTER_METHOD_INIT_VARS;
3636

3737
/* Parse parameters. */
38-
if( zend_parse_method_parameters( ZEND_NUM_ARGS(), getThis(), "On|l",
38+
if( zend_parse_method_parameters( ZEND_NUM_ARGS(), getThis(), "Oz|l",
3939
&object, NumberFormatter_ce_ptr, &number, &type ) == FAILURE )
4040
{
4141
RETURN_THROWS();
@@ -53,10 +53,22 @@ PHP_FUNCTION( numfmt_format )
5353
case IS_DOUBLE:
5454
type = FORMAT_TYPE_DOUBLE;
5555
break;
56-
EMPTY_SWITCH_DEFAULT_CASE();
56+
case IS_STRING:
57+
type = FORMAT_TYPE_DECIMAL;
58+
break;
59+
default:
60+
zend_argument_type_error(1, "must be of type int|float|string, %s given", zend_zval_type_name(number));
5761
}
5862
}
5963

64+
// Avoid losing precision on 32-bit platforms where PHP's "long" isn't
65+
// as long as the FORMAT_TYPE_INT64 which is requested.
66+
#if SIZEOF_ZEND_LONG < 8
67+
if (Z_TYPE_P(number) == IS_STRING && type == FORMAT_TYPE_INT64) {
68+
type = FORMAT_TYPE_DECIMAL;
69+
}
70+
#endif
71+
6072
switch(type) {
6173
case FORMAT_TYPE_INT32:
6274
{
@@ -116,6 +128,24 @@ PHP_FUNCTION( numfmt_format )
116128
}
117129
INTL_METHOD_CHECK_STATUS( nfo, "Number formatting failed" );
118130
break;
131+
132+
case FORMAT_TYPE_DECIMAL:
133+
if (!try_convert_to_string(number)) {
134+
RETURN_THROWS();
135+
}
136+
// Convert string as a DecimalNumber, so we don't lose precision
137+
formatted_len = unum_formatDecimal(FORMATTER_OBJECT(nfo), Z_STRVAL_P(number), Z_STRLEN_P(number), formatted, formatted_len, NULL, &INTL_DATA_ERROR_CODE(nfo));
138+
if (INTL_DATA_ERROR_CODE(nfo) == U_BUFFER_OVERFLOW_ERROR) {
139+
intl_error_reset(INTL_DATA_ERROR_P(nfo));
140+
formatted = eumalloc(formatted_len);
141+
unum_formatDecimal(FORMATTER_OBJECT(nfo), Z_STRVAL_P(number), Z_STRLEN_P(number), formatted, formatted_len, NULL, &INTL_DATA_ERROR_CODE(nfo));
142+
if (U_FAILURE( INTL_DATA_ERROR_CODE(nfo) ) ) {
143+
efree(formatted);
144+
}
145+
}
146+
INTL_METHOD_CHECK_STATUS( nfo, "Number formatting failed" );
147+
break;
148+
119149
case FORMAT_TYPE_CURRENCY:
120150
if (getThis()) {
121151
const char *space;

ext/intl/formatter/formatter_format.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,6 @@
2222
#define FORMAT_TYPE_INT64 2
2323
#define FORMAT_TYPE_DOUBLE 3
2424
#define FORMAT_TYPE_CURRENCY 4
25+
#define FORMAT_TYPE_DECIMAL 5
2526

2627
#endif // FORMATTER_FORMAT_H

ext/intl/php_intl.stub.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ function datefmt_get_error_message(IntlDateFormatter $formatter): string {}
390390

391391
function numfmt_create(string $locale, int $style, ?string $pattern = null): ?NumberFormatter {}
392392

393-
function numfmt_format(NumberFormatter $formatter, int|float $num, int $type = NumberFormatter::TYPE_DEFAULT): string|false {}
393+
function numfmt_format(NumberFormatter $formatter, int|float|string $num, int $type = NumberFormatter::TYPE_DEFAULT): string|false {}
394394

395395
/** @param int $offset */
396396
function numfmt_parse(NumberFormatter $formatter, string $string, int $type = NumberFormatter::TYPE_DOUBLE, &$offset = null): int|float|false {}

ext/intl/php_intl_arginfo.h

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

ext/intl/tests/bug48227.phpt

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,50 @@ intl
55
--FILE--
66
<?php
77

8+
$testcases = ['', 1, NULL, true, false, [], (object)[]];
9+
10+
echo("OOP\n");
811
$x = new NumberFormatter('en_US', NumberFormatter::DECIMAL);
9-
foreach (['', 1, NULL, $x] as $value) {
12+
foreach (array_merge($testcases, [$x]) as $value) {
1013
try {
1114
var_dump($x->format($value));
1215
} catch (TypeError $ex) {
1316
echo $ex->getMessage(), PHP_EOL;
1417
}
1518
}
1619

20+
echo("\nProcedural\n");
21+
$x = numfmt_create('en_US', NumberFormatter::DECIMAL);
22+
foreach (array_merge($testcases, [$x]) as $value) {
23+
try {
24+
var_dump(numfmt_format($x, $value));
25+
} catch (TypeError $ex) {
26+
echo $ex->getMessage(), PHP_EOL;
27+
}
28+
}
29+
1730
?>
1831
--EXPECTF--
19-
NumberFormatter::format(): Argument #1 ($num) must be of type int|float, string given
32+
OOP
33+
bool(false)
2034
string(1) "1"
2135

22-
Deprecated: NumberFormatter::format(): Passing null to parameter #1 ($num) of type int|float is deprecated in %s on line %d
36+
Deprecated: NumberFormatter::format(): Passing null to parameter #1 ($num) of type string|int|float is deprecated in %s on line %d
37+
string(1) "0"
38+
string(1) "1"
39+
string(1) "0"
40+
NumberFormatter::format(): Argument #1 ($num) must be of type string|int|float, array given
41+
NumberFormatter::format(): Argument #1 ($num) must be of type string|int|float, stdClass given
42+
NumberFormatter::format(): Argument #1 ($num) must be of type string|int|float, NumberFormatter given
43+
44+
Non-OOP
45+
bool(false)
46+
string(1) "1"
47+
48+
Deprecated: numfmt_format(): Passing null to parameter #2 ($num) of type string|int|float is deprecated in %s on line %d
49+
string(1) "0"
50+
string(1) "1"
2351
string(1) "0"
24-
NumberFormatter::format(): Argument #1 ($num) must be of type int|float, NumberFormatter given
52+
numfmt_format(): Argument #2 ($num) must be of type string|int|float, array given
53+
numfmt_format(): Argument #2 ($num) must be of type string|int|float, stdClass given
54+
numfmt_format(): Argument #2 ($num) must be of type string|int|float, NumberFormatter given

ext/intl/tests/bug76093.phpt

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
--TEST--
2+
Bug #76093 (NumberFormatter::format loses precision)
3+
--EXTENSIONS--
4+
intl
5+
--FILE--
6+
<?php
7+
8+
# See also https://phabricator.wikimedia.org/T268456
9+
$x = new NumberFormatter('en_US', NumberFormatter::DECIMAL);
10+
foreach ([
11+
'999999999999999999', # Fits in signed 64-bit integer
12+
'9999999999999999999', # Does not fit in signed 64-bit integer
13+
9999999999999999999, # Precision loss seen when passing as number
14+
] as $value) {
15+
try {
16+
var_dump([
17+
'input' => $value,
18+
'default' => $x->format($value),
19+
# Note that TYPE_INT64 isn't actually guaranteed to have an
20+
# 64-bit integer as input, because PHP on 32-bit platforms only
21+
# has 32-bit integers. If you pass the value as a string, PHP
22+
# will use the TYPE_DECIMAL type in order to extend the range.
23+
# Also, casting from double to int64 when the int64 range
24+
# is exceeded results in an implementation-defined value.
25+
'int64' => $x->format($value, NumberFormatter::TYPE_INT64),
26+
'double' => $x->format($value, NumberFormatter::TYPE_DOUBLE),
27+
'decimal' => $x->format($value, NumberFormatter::TYPE_DECIMAL),
28+
]);
29+
} catch (TypeError $ex) {
30+
echo $ex->getMessage(), PHP_EOL;
31+
}
32+
}
33+
34+
?>
35+
--EXPECTF--
36+
array(5) {
37+
["input"]=>
38+
string(18) "999999999999999999"
39+
["default"]=>
40+
string(23) "999,999,999,999,999,999"
41+
["int64"]=>
42+
string(23) "999,999,999,999,999,999"
43+
["double"]=>
44+
string(25) "1,000,000,000,000,000,000"
45+
["decimal"]=>
46+
string(23) "999,999,999,999,999,999"
47+
}
48+
49+
Deprecated: Implicit conversion from float-string "9999999999999999999" to int loses precision in %s on line %d
50+
array(5) {
51+
["input"]=>
52+
string(19) "9999999999999999999"
53+
["default"]=>
54+
string(25) "9,999,999,999,999,999,999"
55+
["int64"]=>
56+
string(%d) "%r9,223,372,036,854,775,807|9,999,999,999,999,999,999%r"
57+
["double"]=>
58+
string(26) "10,000,000,000,000,000,000"
59+
["decimal"]=>
60+
string(25) "9,999,999,999,999,999,999"
61+
}
62+
63+
Deprecated: Implicit conversion from float 1.0E+19 to int loses precision in %s on line %d
64+
array(5) {
65+
["input"]=>
66+
float(1.0E+19)
67+
["default"]=>
68+
string(26) "10,000,000,000,000,000,000"
69+
["int64"]=>
70+
string(%d) "%r[+-]?[0-9,]+%r"
71+
["double"]=>
72+
string(26) "10,000,000,000,000,000,000"
73+
["decimal"]=>
74+
string(26) "10,000,000,000,000,000,000"
75+
}

0 commit comments

Comments
 (0)