Prevent decimal int precision loss in number_format()

Closes GH-11584
php · Jul 13, 2023 · 591f3f6 · 591f3f6
1 parent d17069e
commit 591f3f6
Show file tree

Hide file tree

Showing 6 changed files with 499 additions and 4 deletions.
diff --git a/NEWS b/NEWS
@@ -40,6 +40,8 @@ PHP                                                                        NEWS
 - Standard:
   . Added support for rounding negative places in number_format().
     (Marc Bennewitz)
+  . Prevent precision loss on formatting decimal integers in number_format().
+    (Marc Bennewitz)
   . Added usage of posix_spawn for proc_open when supported by OS. 
     (Cristian Rodriguez)
 

diff --git a/ext/standard/math.c b/ext/standard/math.c
@@ -1130,16 +1130,134 @@ PHPAPI zend_string *_php_math_number_format_ex(double d, int dec, const char *de
 	return res;
 }
 
+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)
+{
+	static const zend_ulong powers[] = {
+		1, 10, 100, 1000, 10000,
+		100000, 1000000, 10000000, 100000000, 1000000000,
+#if SIZEOF_ZEND_LONG == 8
+		10000000000, 100000000000, 1000000000000, 10000000000000, 100000000000000,
+		1000000000000000, 10000000000000000, 100000000000000000, 1000000000000000000, 10000000000000000000ul
+#elif SIZEOF_ZEND_LONG > 8
+# error "Unknown SIZEOF_ZEND_LONG"
+#endif
+	};
+
+	int is_negative = 0;
+	zend_ulong tmpnum;
+	zend_ulong power;
+	zend_ulong power_half;
+	zend_ulong rest;
+
+	zend_string *tmpbuf;
+	zend_string *res;
+	size_t reslen;
+	char *s, *t;  /* source, target */
+	int count = 0;
+	size_t topad;
+
+	// unsigned absolute number and memorize negative sign
+	if (num < 0) {
+		is_negative = 1;
+		tmpnum = ((zend_ulong)-(num + 1)) + 1;
+	} else {
+		tmpnum = (zend_ulong)num;
+	}
+
+	// rounding the number
+	if (dec < 0) {
+		// Check rounding to more negative places than possible
+		if (dec < -(sizeof(powers) / sizeof(powers[0]) - 1)) {
+			tmpnum = 0;
+		} else {
+			power = powers[-dec];
+			power_half = power / 2;
+			rest = tmpnum % power;
+			tmpnum = tmpnum / power;
+
+			if (rest >= power_half) {
+				tmpnum = tmpnum * power + power;
+			} else {
+				tmpnum = tmpnum * power;
+			}
+		}
+
+		// prevent resulting in negative zero
+		if (tmpnum == 0) {
+			is_negative = 0;
+		}
+	}
+
+	tmpbuf = strpprintf(0, ZEND_ULONG_FMT, tmpnum);
+	reslen = ZSTR_LEN(tmpbuf);
+
+	/* allow for thousand separators */
+	if (thousand_sep) {
+		reslen = zend_safe_addmult((reslen-1)/3, thousand_sep_len, reslen, "number formatting");
+	}
+
+	reslen += is_negative;
+
+	if (dec > 0) {
+		reslen += dec;
+
+		if (dec_point) {
+			reslen = zend_safe_addmult(reslen, 1, dec_point_len, "number formatting");
+		}
+	}
+
+	res = zend_string_alloc(reslen, 0);
+
+	s = ZSTR_VAL(tmpbuf) + ZSTR_LEN(tmpbuf) - 1;
+	t = ZSTR_VAL(res) + reslen;
+	*t-- = '\0';
+
+	/* copy the decimal places. */
+	if (dec > 0) {
+		topad = (size_t)dec;
+
+		/* pad with '0's */
+		while (topad--) {
+			*t-- = '0';
+		}
+
+		/* add decimal point */
+		if (dec_point) {
+			t -= dec_point_len;
+			memcpy(t + 1, dec_point, dec_point_len);
+		}
+	}
+
+	/* copy the numbers before the decimal point, adding thousand
+	 * separator every three digits */
+	while (s >= ZSTR_VAL(tmpbuf)) {
+		*t-- = *s--;
+		if (thousand_sep && (++count % 3) == 0 && s >= ZSTR_VAL(tmpbuf)) {
+			t -= thousand_sep_len;
+			memcpy(t + 1, thousand_sep, thousand_sep_len);
+		}
+	}
+
+	if (is_negative) {
+		*t-- = '-';
+	}
+
+	ZSTR_LEN(res) = reslen;
+	zend_string_release_ex(tmpbuf, 0);
+	return res;
+}
+
 /* {{{ Formats a number with grouped thousands */
 PHP_FUNCTION(number_format)
 {
-	double num;
+	zval* num;
 	zend_long dec = 0;
 	char *thousand_sep = NULL, *dec_point = NULL;
 	size_t thousand_sep_len = 0, dec_point_len = 0;
 
 	ZEND_PARSE_PARAMETERS_START(1, 4)
-		Z_PARAM_DOUBLE(num)
+		Z_PARAM_NUMBER(num)
 		Z_PARAM_OPTIONAL
 		Z_PARAM_LONG(dec)
 		Z_PARAM_STRING_OR_NULL(dec_point, dec_point_len)
@@ -1155,7 +1273,17 @@ PHP_FUNCTION(number_format)
 		thousand_sep_len = 1;
 	}
 
-	RETURN_STR(_php_math_number_format_ex(num, (int)dec, dec_point, dec_point_len, thousand_sep, thousand_sep_len));
+	switch (Z_TYPE_P(num)) {
+		case IS_LONG:
+			RETURN_STR(_php_math_number_format_long(Z_LVAL_P(num), dec, dec_point, dec_point_len, thousand_sep, thousand_sep_len));
+			break;
+
+		case IS_DOUBLE:
+			RETURN_STR(_php_math_number_format_ex(Z_DVAL_P(num), (int)dec, dec_point, dec_point_len, thousand_sep, thousand_sep_len));
+			break;
+
+		EMPTY_SWITCH_DEFAULT_CASE()
+	}
 }
 /* }}} */
 

diff --git a/ext/standard/php_math.h b/ext/standard/php_math.h
@@ -21,6 +21,7 @@
 PHPAPI double _php_math_round(double value, int places, int mode);
 PHPAPI zend_string *_php_math_number_format(double d, int dec, char dec_point, char thousand_sep);
 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);
+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);
 PHPAPI zend_string * _php_math_longtobase(zend_long arg, int base);
 PHPAPI zend_long _php_math_basetolong(zval *arg, int base);
 PHPAPI void _php_math_basetozval(zend_string *str, int base, zval *ret);

diff --git a/ext/standard/tests/math/number_format_basic.phpt b/ext/standard/tests/math/number_format_basic.phpt
@@ -6,6 +6,10 @@ $values = array(1234.5678,
                 -1234.5678,
                 1234.6578e4,
                 -1234.56789e4,
+                999999,
+                -999999,
+                999999.0,
+                -999999.0,
                 0x1234CDEF,
                 02777777777,
                 "123456789",
@@ -37,13 +41,23 @@ for ($i = 0; $i < count($values); $i++) {
     $res = number_format($values[$i], 2, ',' , ' ');
     var_dump($res);
 }
+
+echo "\n number_format tests.....multichar format\n";
+for ($i = 0; $i < count($values); $i++) {
+    $res = number_format($values[$i], 2, ' DECIMALS ' , ' THOUSAND ');
+    var_dump($res);
+}
 ?>
 --EXPECT--
 number_format tests.....default
 string(5) "1,235"
 string(6) "-1,235"
 string(10) "12,346,578"
 string(11) "-12,345,679"
+string(7) "999,999"
+string(8) "-999,999"
+string(7) "999,999"
+string(8) "-999,999"
 string(11) "305,450,479"
 string(11) "402,653,183"
 string(11) "123,456,789"
@@ -57,6 +71,10 @@ string(8) "1,234.57"
 string(9) "-1,234.57"
 string(13) "12,346,578.00"
 string(14) "-12,345,678.90"
+string(10) "999,999.00"
+string(11) "-999,999.00"
+string(10) "999,999.00"
+string(11) "-999,999.00"
 string(14) "305,450,479.00"
 string(14) "402,653,183.00"
 string(14) "123,456,789.00"
@@ -70,6 +88,10 @@ string(8) "1 234.57"
 string(9) "-1 234.57"
 string(13) "12 346 578.00"
 string(14) "-12 345 678.90"
+string(10) "999 999.00"
+string(11) "-999 999.00"
+string(10) "999 999.00"
+string(11) "-999 999.00"
 string(14) "305 450 479.00"
 string(14) "402 653 183.00"
 string(14) "123 456 789.00"
@@ -83,10 +105,31 @@ string(8) "1 234,57"
 string(9) "-1 234,57"
 string(13) "12 346 578,00"
 string(14) "-12 345 678,90"
+string(10) "999 999,00"
+string(11) "-999 999,00"
+string(10) "999 999,00"
+string(11) "-999 999,00"
 string(14) "305 450 479,00"
 string(14) "402 653 183,00"
 string(14) "123 456 789,00"
 string(6) "123,46"
 string(6) "123,46"
 string(4) "1,00"
 string(4) "0,00"
+
+ number_format tests.....multichar format
+string(26) "1 THOUSAND 234 DECIMALS 57"
+string(27) "-1 THOUSAND 234 DECIMALS 57"
+string(40) "12 THOUSAND 346 THOUSAND 578 DECIMALS 00"
+string(41) "-12 THOUSAND 345 THOUSAND 678 DECIMALS 90"
+string(28) "999 THOUSAND 999 DECIMALS 00"
+string(29) "-999 THOUSAND 999 DECIMALS 00"
+string(28) "999 THOUSAND 999 DECIMALS 00"
+string(29) "-999 THOUSAND 999 DECIMALS 00"
+string(41) "305 THOUSAND 450 THOUSAND 479 DECIMALS 00"
+string(41) "402 THOUSAND 653 THOUSAND 183 DECIMALS 00"
+string(41) "123 THOUSAND 456 THOUSAND 789 DECIMALS 00"
+string(15) "123 DECIMALS 46"
+string(15) "123 DECIMALS 46"
+string(13) "1 DECIMALS 00"
+string(13) "0 DECIMALS 00"