Skip to content

Commit 591f3f6

Browse files
marc-mabebukka
marc-mabe
authored and
bukka
committed
Prevent decimal int precision loss in number_format()
Closes GH-11584
1 parent d17069e commit 591f3f6

File tree

6 files changed

+499
-4
lines changed

6 files changed

+499
-4
lines changed

‎NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ PHP NEWS
4040
- Standard:
4141
. Added support for rounding negative places in number_format().
4242
(Marc Bennewitz)
43+
. Prevent precision loss on formatting decimal integers in number_format().
44+
(Marc Bennewitz)
4345
. Added usage of posix_spawn for proc_open when supported by OS.
4446
(Cristian Rodriguez)
4547

‎ext/standard/math.c

Lines changed: 131 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,16 +1130,134 @@ PHPAPI zend_string *_php_math_number_format_ex(double d, int dec, const char *de
11301130
return res;
11311131
}
11321132

1133+
PHPAPI zend_string *_php_math_number_format_long(zend_long num, zend_long dec, const char *dec_point,
1134+
size_t dec_point_len, const char *thousand_sep, size_t thousand_sep_len)
1135+
{
1136+
static const zend_ulong powers[] = {
1137+
1, 10, 100, 1000, 10000,
1138+
100000, 1000000, 10000000, 100000000, 1000000000,
1139+
#if SIZEOF_ZEND_LONG == 8
1140+
10000000000, 100000000000, 1000000000000, 10000000000000, 100000000000000,
1141+
1000000000000000, 10000000000000000, 100000000000000000, 1000000000000000000, 10000000000000000000ul
1142+
#elif SIZEOF_ZEND_LONG > 8
1143+
# error "Unknown SIZEOF_ZEND_LONG"
1144+
#endif
1145+
};
1146+
1147+
int is_negative = 0;
1148+
zend_ulong tmpnum;
1149+
zend_ulong power;
1150+
zend_ulong power_half;
1151+
zend_ulong rest;
1152+
1153+
zend_string *tmpbuf;
1154+
zend_string *res;
1155+
size_t reslen;
1156+
char *s, *t; /* source, target */
1157+
int count = 0;
1158+
size_t topad;
1159+
1160+
// unsigned absolute number and memorize negative sign
1161+
if (num < 0) {
1162+
is_negative = 1;
1163+
tmpnum = ((zend_ulong)-(num + 1)) + 1;
1164+
} else {
1165+
tmpnum = (zend_ulong)num;
1166+
}
1167+
1168+
// rounding the number
1169+
if (dec < 0) {
1170+
// Check rounding to more negative places than possible
1171+
if (dec < -(sizeof(powers) / sizeof(powers[0]) - 1)) {
1172+
tmpnum = 0;
1173+
} else {
1174+
power = powers[-dec];
1175+
power_half = power / 2;
1176+
rest = tmpnum % power;
1177+
tmpnum = tmpnum / power;
1178+
1179+
if (rest >= power_half) {
1180+
tmpnum = tmpnum * power + power;
1181+
} else {
1182+
tmpnum = tmpnum * power;
1183+
}
1184+
}
1185+
1186+
// prevent resulting in negative zero
1187+
if (tmpnum == 0) {
1188+
is_negative = 0;
1189+
}
1190+
}
1191+
1192+
tmpbuf = strpprintf(0, ZEND_ULONG_FMT, tmpnum);
1193+
reslen = ZSTR_LEN(tmpbuf);
1194+
1195+
/* allow for thousand separators */
1196+
if (thousand_sep) {
1197+
reslen = zend_safe_addmult((reslen-1)/3, thousand_sep_len, reslen, "number formatting");
1198+
}
1199+
1200+
reslen += is_negative;
1201+
1202+
if (dec > 0) {
1203+
reslen += dec;
1204+
1205+
if (dec_point) {
1206+
reslen = zend_safe_addmult(reslen, 1, dec_point_len, "number formatting");
1207+
}
1208+
}
1209+
1210+
res = zend_string_alloc(reslen, 0);
1211+
1212+
s = ZSTR_VAL(tmpbuf) + ZSTR_LEN(tmpbuf) - 1;
1213+
t = ZSTR_VAL(res) + reslen;
1214+
*t-- = '\0';
1215+
1216+
/* copy the decimal places. */
1217+
if (dec > 0) {
1218+
topad = (size_t)dec;
1219+
1220+
/* pad with '0's */
1221+
while (topad--) {
1222+
*t-- = '0';
1223+
}
1224+
1225+
/* add decimal point */
1226+
if (dec_point) {
1227+
t -= dec_point_len;
1228+
memcpy(t + 1, dec_point, dec_point_len);
1229+
}
1230+
}
1231+
1232+
/* copy the numbers before the decimal point, adding thousand
1233+
* separator every three digits */
1234+
while (s >= ZSTR_VAL(tmpbuf)) {
1235+
*t-- = *s--;
1236+
if (thousand_sep && (++count % 3) == 0 && s >= ZSTR_VAL(tmpbuf)) {
1237+
t -= thousand_sep_len;
1238+
memcpy(t + 1, thousand_sep, thousand_sep_len);
1239+
}
1240+
}
1241+
1242+
if (is_negative) {
1243+
*t-- = '-';
1244+
}
1245+
1246+
ZSTR_LEN(res) = reslen;
1247+
zend_string_release_ex(tmpbuf, 0);
1248+
return res;
1249+
}
1250+
11331251
/* {{{ Formats a number with grouped thousands */
11341252
PHP_FUNCTION(number_format)
11351253
{
1136-
double num;
1254+
zval* num;
11371255
zend_long dec = 0;
11381256
char *thousand_sep = NULL, *dec_point = NULL;
11391257
size_t thousand_sep_len = 0, dec_point_len = 0;
11401258

11411259
ZEND_PARSE_PARAMETERS_START(1, 4)
1142-
Z_PARAM_DOUBLE(num)
1260+
Z_PARAM_NUMBER(num)
11431261
Z_PARAM_OPTIONAL
11441262
Z_PARAM_LONG(dec)
11451263
Z_PARAM_STRING_OR_NULL(dec_point, dec_point_len)
@@ -1155,7 +1273,17 @@ PHP_FUNCTION(number_format)
11551273
thousand_sep_len = 1;
11561274
}
11571275

1158-
RETURN_STR(_php_math_number_format_ex(num, (int)dec, dec_point, dec_point_len, thousand_sep, thousand_sep_len));
1276+
switch (Z_TYPE_P(num)) {
1277+
case IS_LONG:
1278+
RETURN_STR(_php_math_number_format_long(Z_LVAL_P(num), dec, dec_point, dec_point_len, thousand_sep, thousand_sep_len));
1279+
break;
1280+
1281+
case IS_DOUBLE:
1282+
RETURN_STR(_php_math_number_format_ex(Z_DVAL_P(num), (int)dec, dec_point, dec_point_len, thousand_sep, thousand_sep_len));
1283+
break;
1284+
1285+
EMPTY_SWITCH_DEFAULT_CASE()
1286+
}
11591287
}
11601288
/* }}} */
11611289

‎ext/standard/php_math.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
PHPAPI double _php_math_round(double value, int places, int mode);
2222
PHPAPI zend_string *_php_math_number_format(double d, int dec, char dec_point, char thousand_sep);
2323
PHPAPI zend_string *_php_math_number_format_ex(double d, int dec, const char *dec_point, size_t dec_point_len, const char *thousand_sep, size_t thousand_sep_len);
24+
PHPAPI zend_string *_php_math_number_format_long(zend_long num, zend_long dec, const char *dec_point, size_t dec_point_len, const char *thousand_sep, size_t thousand_sep_len);
2425
PHPAPI zend_string * _php_math_longtobase(zend_long arg, int base);
2526
PHPAPI zend_long _php_math_basetolong(zval *arg, int base);
2627
PHPAPI void _php_math_basetozval(zend_string *str, int base, zval *ret);

‎ext/standard/tests/math/number_format_basic.phpt

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ $values = array(1234.5678,
66
-1234.5678,
77
1234.6578e4,
88
-1234.56789e4,
9+
999999,
10+
-999999,
11+
999999.0,
12+
-999999.0,
913
0x1234CDEF,
1014
02777777777,
1115
"123456789",
@@ -37,13 +41,23 @@ for ($i = 0; $i < count($values); $i++) {
3741
$res = number_format($values[$i], 2, ',' , ' ');
3842
var_dump($res);
3943
}
44+
45+
echo "\n number_format tests.....multichar format\n";
46+
for ($i = 0; $i < count($values); $i++) {
47+
$res = number_format($values[$i], 2, ' DECIMALS ' , ' THOUSAND ');
48+
var_dump($res);
49+
}
4050
?>
4151
--EXPECT--
4252
number_format tests.....default
4353
string(5) "1,235"
4454
string(6) "-1,235"
4555
string(10) "12,346,578"
4656
string(11) "-12,345,679"
57+
string(7) "999,999"
58+
string(8) "-999,999"
59+
string(7) "999,999"
60+
string(8) "-999,999"
4761
string(11) "305,450,479"
4862
string(11) "402,653,183"
4963
string(11) "123,456,789"
@@ -57,6 +71,10 @@ string(8) "1,234.57"
5771
string(9) "-1,234.57"
5872
string(13) "12,346,578.00"
5973
string(14) "-12,345,678.90"
74+
string(10) "999,999.00"
75+
string(11) "-999,999.00"
76+
string(10) "999,999.00"
77+
string(11) "-999,999.00"
6078
string(14) "305,450,479.00"
6179
string(14) "402,653,183.00"
6280
string(14) "123,456,789.00"
@@ -70,6 +88,10 @@ string(8) "1 234.57"
7088
string(9) "-1 234.57"
7189
string(13) "12 346 578.00"
7290
string(14) "-12 345 678.90"
91+
string(10) "999 999.00"
92+
string(11) "-999 999.00"
93+
string(10) "999 999.00"
94+
string(11) "-999 999.00"
7395
string(14) "305 450 479.00"
7496
string(14) "402 653 183.00"
7597
string(14) "123 456 789.00"
@@ -83,10 +105,31 @@ string(8) "1 234,57"
83105
string(9) "-1 234,57"
84106
string(13) "12 346 578,00"
85107
string(14) "-12 345 678,90"
108+
string(10) "999 999,00"
109+
string(11) "-999 999,00"
110+
string(10) "999 999,00"
111+
string(11) "-999 999,00"
86112
string(14) "305 450 479,00"
87113
string(14) "402 653 183,00"
88114
string(14) "123 456 789,00"
89115
string(6) "123,46"
90116
string(6) "123,46"
91117
string(4) "1,00"
92118
string(4) "0,00"
119+
120+
number_format tests.....multichar format
121+
string(26) "1 THOUSAND 234 DECIMALS 57"
122+
string(27) "-1 THOUSAND 234 DECIMALS 57"
123+
string(40) "12 THOUSAND 346 THOUSAND 578 DECIMALS 00"
124+
string(41) "-12 THOUSAND 345 THOUSAND 678 DECIMALS 90"
125+
string(28) "999 THOUSAND 999 DECIMALS 00"
126+
string(29) "-999 THOUSAND 999 DECIMALS 00"
127+
string(28) "999 THOUSAND 999 DECIMALS 00"
128+
string(29) "-999 THOUSAND 999 DECIMALS 00"
129+
string(41) "305 THOUSAND 450 THOUSAND 479 DECIMALS 00"
130+
string(41) "402 THOUSAND 653 THOUSAND 183 DECIMALS 00"
131+
string(41) "123 THOUSAND 456 THOUSAND 789 DECIMALS 00"
132+
string(15) "123 DECIMALS 46"
133+
string(15) "123 DECIMALS 46"
134+
string(13) "1 DECIMALS 00"
135+
string(13) "0 DECIMALS 00"

0 commit comments

Comments
 (0)