From fbb1f054a5e3d4ed99b8838ffbd91b6586be7503 Mon Sep 17 00:00:00 2001 From: Saki Takamachi Date: Thu, 5 Dec 2024 23:57:22 +0900 Subject: [PATCH 1/4] Correctly compare 0 and -0 --- NEWS | 3 +- ext/bcmath/libbcmath/src/compare.c | 8 +++ ext/bcmath/libbcmath/src/str2num.c | 9 +++ ext/bcmath/tests/bccomp_variation002.phpt | 2 + .../number/methods/compare_near_zero.phpt | 71 +++++++++++++++++++ 5 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 ext/bcmath/tests/number/methods/compare_near_zero.phpt diff --git a/NEWS b/NEWS index 7f59f831c9126..81c632d333f58 100644 --- a/NEWS +++ b/NEWS @@ -12,8 +12,9 @@ PHP NEWS 05 Dec 2024, PHP 8.4.2 - BcMath: - . Fixed bug GH-16978 (Avoid unnecessary padding with leading zeros) + . Fixed bug GH-16978 (Avoid unnecessary padding with leading zeros). (Saki Takamachi) + . Fixed bug GH-17049 (Correctly compare 0 and -0). (Saki Takamachi) - COM: . Fixed bug GH-16991 (Getting typeinfo of non DISPATCH variant segfaults). diff --git a/ext/bcmath/libbcmath/src/compare.c b/ext/bcmath/libbcmath/src/compare.c index f2df5cc24c57e..c17efdc63764c 100644 --- a/ext/bcmath/libbcmath/src/compare.c +++ b/ext/bcmath/libbcmath/src/compare.c @@ -45,6 +45,14 @@ bcmath_compare_result _bc_do_compare(bc_num n1, bc_num n2, size_t scale, bool us /* First, compare signs. */ if (use_sign && n1->n_sign != n2->n_sign) { + if (n1->n_len == 1 && n2->n_len == 1 && + n1->n_value[0] == 0 && n2->n_value[0] == 0 && + (n1->n_scale > scale || n2->n_scale > scale) && + bc_is_zero_for_scale(n1, scale) && bc_is_zero_for_scale(n2, scale) + ) { + /* e.g. 0.00 <=> -0.00 */ + return BCMATH_EQUAL; + } if (n1->n_sign == PLUS) { /* Positive N1 > Negative N2 */ return BCMATH_LEFT_GREATER; diff --git a/ext/bcmath/libbcmath/src/str2num.c b/ext/bcmath/libbcmath/src/str2num.c index a56fefa02a4c7..79c1d4216fe7b 100644 --- a/ext/bcmath/libbcmath/src/str2num.c +++ b/ext/bcmath/libbcmath/src/str2num.c @@ -173,6 +173,15 @@ bool bc_str2num(bc_num *num, const char *str, const char *end, size_t scale, siz if (str_scale > scale && !auto_scale) { fractional_end -= str_scale - scale; str_scale = scale; + + /* + * e.g. 123.0001 with scale 2 -> 123.00 + * So, remove the trailing 0 again. + */ + if (str_scale > 0) { + const char *fractional_new_end = bc_skip_zero_reverse(fractional_end, fractional_ptr); + str_scale -= fractional_new_end - fractional_end; + } } } else { if (full_scale) { diff --git a/ext/bcmath/tests/bccomp_variation002.phpt b/ext/bcmath/tests/bccomp_variation002.phpt index 299f454780601..145181083570b 100644 --- a/ext/bcmath/tests/bccomp_variation002.phpt +++ b/ext/bcmath/tests/bccomp_variation002.phpt @@ -13,6 +13,7 @@ echo bccomp("-2.29", "2.3", "2") . "\n"; echo bccomp("2.29", "-2.3", "2") . "\n"; echo bccomp("-2.29", "-2.3", "1") . "\n"; echo bccomp("-2.29", "0", "1") . "\n"; +echo bccomp("0.001", "-0.001", "1") . "\n"; ?> --EXPECT-- 0 @@ -22,3 +23,4 @@ echo bccomp("-2.29", "0", "1") . "\n"; 1 1 -1 +0 diff --git a/ext/bcmath/tests/number/methods/compare_near_zero.phpt b/ext/bcmath/tests/number/methods/compare_near_zero.phpt new file mode 100644 index 0000000000000..342b38294790b --- /dev/null +++ b/ext/bcmath/tests/number/methods/compare_near_zero.phpt @@ -0,0 +1,71 @@ +--TEST-- +BcMath\Number compare() with scale +--EXTENSIONS-- +bcmath +--FILE-- + {$value2}: ", 20, ' ', STR_PAD_LEFT); + $output .= str_pad((string) $value1->compare($value2, $scale), 2, ' ', STR_PAD_LEFT); + echo "{$output}\n"; + } + } +} +?> +--EXPECT-- +========== scale is 0 ========== + 0.001 <=> 0: 0 + 0.001 <=> 0: 0 + 0.001 <=> 0.0011: 0 + 0.001 <=> -0.0011: 0 + -0.001 <=> 0: 0 + -0.001 <=> 0: 0 + -0.001 <=> 0.0011: 0 +-0.001 <=> -0.0011: 0 +========== scale is 2 ========== + 0.001 <=> 0: 0 + 0.001 <=> 0: 0 + 0.001 <=> 0.0011: 0 + 0.001 <=> -0.0011: 0 + -0.001 <=> 0: 0 + -0.001 <=> 0: 0 + -0.001 <=> 0.0011: 0 +-0.001 <=> -0.0011: 0 +========== scale is 3 ========== + 0.001 <=> 0: 1 + 0.001 <=> 0: 1 + 0.001 <=> 0.0011: 0 + 0.001 <=> -0.0011: 1 + -0.001 <=> 0: -1 + -0.001 <=> 0: -1 + -0.001 <=> 0.0011: -1 +-0.001 <=> -0.0011: 0 +========== scale is 4 ========== + 0.001 <=> 0: 1 + 0.001 <=> 0: 1 + 0.001 <=> 0.0011: -1 + 0.001 <=> -0.0011: 1 + -0.001 <=> 0: -1 + -0.001 <=> 0: -1 + -0.001 <=> 0.0011: -1 +-0.001 <=> -0.0011: 1 From 40ea004359fb0a90282efa904238eab15169e77b Mon Sep 17 00:00:00 2001 From: Saki Takamachi Date: Fri, 6 Dec 2024 09:34:31 +0900 Subject: [PATCH 2/4] Fixed NEWS --- NEWS | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 81c632d333f58..19d95ae07cf85 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.4.3 +- BcMath: + . Fixed bug GH-17049 (Correctly compare 0 and -0). (Saki Takamachi) + - Iconv: . Fixed bug GH-17047 (UAF on iconv filter failure). (nielsdos) @@ -14,7 +17,6 @@ PHP NEWS - BcMath: . Fixed bug GH-16978 (Avoid unnecessary padding with leading zeros). (Saki Takamachi) - . Fixed bug GH-17049 (Correctly compare 0 and -0). (Saki Takamachi) - COM: . Fixed bug GH-16991 (Getting typeinfo of non DISPATCH variant segfaults). From ef38af0bb7e83b24006c21b82cfd3516e1a96c6e Mon Sep 17 00:00:00 2001 From: Saki Takamachi Date: Fri, 6 Dec 2024 09:35:18 +0900 Subject: [PATCH 3/4] Fixed the order of if conditions --- ext/bcmath/libbcmath/src/compare.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/bcmath/libbcmath/src/compare.c b/ext/bcmath/libbcmath/src/compare.c index c17efdc63764c..56de12fe04940 100644 --- a/ext/bcmath/libbcmath/src/compare.c +++ b/ext/bcmath/libbcmath/src/compare.c @@ -45,9 +45,9 @@ bcmath_compare_result _bc_do_compare(bc_num n1, bc_num n2, size_t scale, bool us /* First, compare signs. */ if (use_sign && n1->n_sign != n2->n_sign) { - if (n1->n_len == 1 && n2->n_len == 1 && + if ((n1->n_scale > scale || n2->n_scale > scale) && + n1->n_len == 1 && n2->n_len == 1 && n1->n_value[0] == 0 && n2->n_value[0] == 0 && - (n1->n_scale > scale || n2->n_scale > scale) && bc_is_zero_for_scale(n1, scale) && bc_is_zero_for_scale(n2, scale) ) { /* e.g. 0.00 <=> -0.00 */ From 9f05c4045b8d06749b00c3c3bb8c4b693d0d6f38 Mon Sep 17 00:00:00 2001 From: Saki Takamachi Date: Sat, 7 Dec 2024 01:47:13 +0900 Subject: [PATCH 4/4] Added a comment --- ext/bcmath/libbcmath/src/compare.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ext/bcmath/libbcmath/src/compare.c b/ext/bcmath/libbcmath/src/compare.c index 56de12fe04940..2c24dab777059 100644 --- a/ext/bcmath/libbcmath/src/compare.c +++ b/ext/bcmath/libbcmath/src/compare.c @@ -45,6 +45,10 @@ bcmath_compare_result _bc_do_compare(bc_num n1, bc_num n2, size_t scale, bool us /* First, compare signs. */ if (use_sign && n1->n_sign != n2->n_sign) { + /* + * scale and n->n_scale differ only in Number objects. + * This happens when explicitly specify the scale in a Number method. + */ if ((n1->n_scale > scale || n2->n_scale > scale) && n1->n_len == 1 && n2->n_len == 1 && n1->n_value[0] == 0 && n2->n_value[0] == 0 &&