ext/bcmath: null should not be supported for operator overloading + comparison issues

ext/bcmath/bcmath.c

@@ -1175,7 +1175,7 @@ static zend_result bcmath_number_parse_num(zval *zv, zend_object **obj, zend_str
 
 			case IS_NULL:
 				*lval = 0;
-				return SUCCESS;
+				return FAILURE;
 
 			default:
 				return zend_parse_arg_str_or_long_slow(zv, str, lval, 1 /* dummy */) ? SUCCESS : FAILURE;
@@ -1234,9 +1234,12 @@ static zend_result bcmath_number_do_operation(uint8_t opcode, zval *ret_val, zva
 	bc_num n2 = NULL;
 	size_t n1_full_scale;
 	size_t n2_full_scale;
-	if (UNEXPECTED(bc_num_from_obj_or_str_or_long(&n1, &n1_full_scale, obj1, str1, lval1) == FAILURE ||
-		bc_num_from_obj_or_str_or_long(&n2, &n2_full_scale, obj2, str2, lval2) == FAILURE)) {
-		zend_value_error("Number is not well-formed");
+	if (UNEXPECTED(bc_num_from_obj_or_str_or_long(&n1, &n1_full_scale, obj1, str1, lval1) == FAILURE)) {
+		zend_value_error("Left string operand cannot be converted to BcMath\\Number");
+		goto fail;
+	}
+	if (UNEXPECTED(bc_num_from_obj_or_str_or_long(&n2, &n2_full_scale, obj2, str2, lval2) == FAILURE)) {
+		zend_value_error("Right string operand cannot be converted to BcMath\\Number");
 		goto fail;
 	}
 
@@ -1317,39 +1320,39 @@ static int bcmath_number_compare(zval *op1, zval *op2)
 	bc_num n1 = NULL;
 	bc_num n2 = NULL;
 
+	int ret = ZEND_UNCOMPARABLE;
+
 	if (UNEXPECTED(bcmath_number_parse_num(op1, &obj1, &str1, &lval1) == FAILURE)) {
-		goto fallback;
+		goto failure;
 	}
 
 	if (UNEXPECTED(bcmath_number_parse_num(op2, &obj2, &str2, &lval2) == FAILURE)) {
-		goto fallback;
+		goto failure;
 	}
 
 	size_t n1_full_scale;
 	size_t n2_full_scale;
 	if (UNEXPECTED(bc_num_from_obj_or_str_or_long(&n1, &n1_full_scale, obj1, str1, lval1) == FAILURE ||
 		bc_num_from_obj_or_str_or_long(&n2, &n2_full_scale, obj2, str2, lval2) == FAILURE)) {
-		goto fallback;
+		goto failure;
 	}
 
 	if (UNEXPECTED(CHECK_SCALE_OVERFLOW(n1_full_scale) || CHECK_SCALE_OVERFLOW(n2_full_scale))) {
 		zend_value_error("scale must be between 0 and %d", INT_MAX);
-		goto fallback;
+		goto failure;
 	}
 
-	bcmath_compare_result ret = bc_compare(n1, n2, MAX(n1->n_scale, n2->n_scale));
+	ret = bc_compare(n1, n2, MAX(n1->n_scale, n2->n_scale));
 
+failure:
 	if (Z_TYPE_P(op1) != IS_OBJECT) {
 		bc_free_num(&n1);
 	}
 	if (Z_TYPE_P(op2) != IS_OBJECT) {
 		bc_free_num(&n2);
 	}
 
-	return (int) ret;
-
-fallback:
-	return zend_std_compare_objects(op1, op2);
+	return ret;
 }
 
 #define BCMATH_PARAM_NUMBER_OR_STR_OR_LONG(dest_obj, ce, dest_str, dest_long) \

ext/bcmath/tests/number/operators/calc_non_numeric_string.phpt

@@ -43,9 +43,9 @@ try {
 }
 ?>
 --EXPECT--
-Number is not well-formed
-Number is not well-formed
-Number is not well-formed
-Number is not well-formed
-Number is not well-formed
-Number is not well-formed
+Right string operand cannot be converted to BcMath\Number
+Right string operand cannot be converted to BcMath\Number
+Right string operand cannot be converted to BcMath\Number
+Right string operand cannot be converted to BcMath\Number
+Right string operand cannot be converted to BcMath\Number
+Right string operand cannot be converted to BcMath\Number

ext/bcmath/tests/number/operators/calc_null.phpt

@@ -0,0 +1,51 @@
+--TEST--
+BcMath\Number calc undefined var by operator
+--EXTENSIONS--
+bcmath
+--FILE--
+<?php
+$num = new BcMath\Number(100);
+
+try {
+    $num + null;
+} catch (Error $e) {
+    echo $e->getMessage() . "\n";
+}
+
+try {
+    $num - null;
+} catch (Error $e) {
+    echo $e->getMessage() . "\n";
+}
+
+try {
+    $num * null;
+} catch (Error $e) {
+    echo $e->getMessage() . "\n";
+}
+
+try {
+    $num / null;
+} catch (Error $e) {
+    echo $e->getMessage() . "\n";
+}
+
+try {
+    $num % null;
+} catch (Error $e) {
+    echo $e->getMessage() . "\n";
+}
+
+try {
+    $num ** null;
+} catch (Error $e) {
+    echo $e->getMessage() . "\n";
+}
+?>
+--EXPECT--
+Unsupported operand types: BcMath\Number + null
+Unsupported operand types: BcMath\Number - null
+Unsupported operand types: BcMath\Number * null
+Unsupported operand types: BcMath\Number / null
+Unsupported operand types: BcMath\Number % null
+Unsupported operand types: BcMath\Number ** null

ext/bcmath/tests/number/operators/calc_undef.phpt

@@ -44,15 +44,19 @@ try {
 ?>
 --EXPECTF--
 Warning: Undefined variable $undef in %s
+Unsupported operand types: BcMath\Number + null
 
 Warning: Undefined variable $undef in %s
+Unsupported operand types: BcMath\Number - null
 
 Warning: Undefined variable $undef in %s
+Unsupported operand types: BcMath\Number * null
 
 Warning: Undefined variable $undef in %s
-Division by zero
+Unsupported operand types: BcMath\Number / null
 
 Warning: Undefined variable $undef in %s
-Modulo by zero
+Unsupported operand types: BcMath\Number % null
 
 Warning: Undefined variable $undef in %s
+Unsupported operand types: BcMath\Number ** null

ext/bcmath/tests/number/operators/compare_with_invalid_types.phpt

@@ -0,0 +1,105 @@
+--TEST--
+BcMath\Number compare by operator with non-sense
+--EXTENSIONS--
+bcmath
+--FILE--
+<?php
+
+$values2 = [
+    [null, 'null'],
+    ['string', 'string'],
+    [new stdClass(), 'object'],
+    [[], 'array'],
+    [STDERR, 'resource'],
+];
+
+$value1 = new BcMath\Number('100.0000');
+
+foreach ($values2 as [$value2, $type2]) {
+    echo "========== with {$type2} ==========\n";
+    echo "{$value1} >  {$type2}: " . ($value1 > $value2 ? 'true' : 'false') . "\n";
+    echo "{$value1} >= {$type2}: " . ($value1 >= $value2 ? 'true' : 'false') . "\n";
+    echo "{$value1} == {$type2}: " . ($value1 == $value2 ? 'true' : 'false') . "\n";
+    echo "{$value1} <= {$type2}: " . ($value1 <= $value2 ? 'true' : 'false') . "\n";
+    echo "{$value1} <  {$type2}: " . ($value1 < $value2 ? 'true' : 'false') . "\n";
+
+    echo "\ninversion\n";
+    echo "{$type2} >  {$value1}: " . ($value2 > $value1 ? 'true' : 'false') . "\n";
+    echo "{$type2} >= {$value1}: " . ($value2 >= $value1 ? 'true' : 'false') . "\n";
+    echo "{$type2} == {$value1}: " . ($value2 == $value1 ? 'true' : 'false') . "\n";
+    echo "{$type2} <= {$value1}: " . ($value2 <= $value1 ? 'true' : 'false') . "\n";
+    echo "{$type2} <  {$value1}: " . ($value2 < $value1 ? 'true' : 'false') . "\n";
+
+    echo "\n";
+}
+?>
+--EXPECT--
+========== with null ==========
+100.0000 >  null: true
+100.0000 >= null: true
+100.0000 == null: false
+100.0000 <= null: false
+100.0000 <  null: false
+
+inversion
+null >  100.0000: false
+null >= 100.0000: false
+null == 100.0000: false
+null <= 100.0000: true
+null <  100.0000: true
+
+========== with string ==========
+100.0000 >  string: false
+100.0000 >= string: false
+100.0000 == string: false
+100.0000 <= string: false
+100.0000 <  string: false
+
+inversion
+string >  100.0000: false
+string >= 100.0000: false
+string == 100.0000: false
+string <= 100.0000: false
+string <  100.0000: false
+
+========== with object ==========
+100.0000 >  object: false
+100.0000 >= object: false
+100.0000 == object: false
+100.0000 <= object: false
+100.0000 <  object: false
+
+inversion
+object >  100.0000: false
+object >= 100.0000: false
+object == 100.0000: false
+object <= 100.0000: false
+object <  100.0000: false
+
+========== with array ==========
+100.0000 >  array: false
+100.0000 >= array: false
+100.0000 == array: false
+100.0000 <= array: false
+100.0000 <  array: false
+
+inversion
+array >  100.0000: false
+array >= 100.0000: false
+array == 100.0000: false
+array <= 100.0000: false
+array <  100.0000: false
+
+========== with resource ==========
+100.0000 >  resource: false
+100.0000 >= resource: false
+100.0000 == resource: false
+100.0000 <= resource: false
+100.0000 <  resource: false
+
+inversion
+resource >  100.0000: false
+resource >= 100.0000: false
+resource == 100.0000: false
+resource <= 100.0000: false
+resource <  100.0000: false