php-decimal / ext-decimal

Correctly-rounded, arbitrary precision decimal floating-point arithmetic in PHP
https://php-decimal.github.io/
MIT License
336 stars 19 forks source link

Negative places in Decimal::toFixed causes memory allocation error #52

Closed sh41 closed 3 years ago

sh41 commented 3 years ago

First up, thanks for this extension. It has made my life so much easier since I moved all of my code from bcmath to this.

I recently ran into a bug and am posting here to hopefully save someone time in the future.

Steps to reproduce:

docker run -it php:7.4.12-cli-buster bash
apt-get update
apt-get install -y libmpdec-dev
pecl install decimal
php -d "extension=decimal.so" -r '(new Decimal\Decimal(1))->toFixed(-1);'

Result:

Fatal error: Failed to allocate memory for decimal in Command line code on line 1

You could also use this script to reproduce if you already have a working installation of php with ext-decimal

<?php

$d = new Decimal\Decimal(1);
$d->toFixed(-1);

which outputs:

Fatal error: Failed to allocate memory for decimal in /path/decimaltest.php on line 4

I understand that passing a negative number to Decimal::toFixed is incorrect, but an uncatchable Fatal error with a message about memory allocation made tracking down the bug quite difficult. My suggestion, if anyone has the time or inclination to look at this, is to have a clearer error message here that the argument of -1 is invalid.

Thanks again

rtheunissen commented 3 years ago

@sh41 thank you for bringing this to the table. There should at the very least be range validation for a clearer, more controlled error state. I don't think there is an intuitive meaning of a negative number here.

rtheunissen commented 3 years ago

This has been fixed and released as part of v1.4.0