Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

number_format function gives an out of memory error for bad $decimals value #17384

Open
phpfui opened this issue Jan 6, 2025 · 8 comments
Open

Comments

@phpfui
Copy link

phpfui commented Jan 6, 2025

Description

The following code:

number_format(1.23456, 9876543210);

Resulted in this output:

Fatal error: Allowed memory size of 1342177280 bytes exhausted (tried to allocate 2147483680 bytes)

But I expected this output instead:

Anything but a out of memory error.

This is obviously not a high priority issue, as the $decimal parameter is invalid, but thought I would report it.

It produces the output of "1" in PHP versions 8.0.30 and 7.4.33. I would consider this an acceptable value. Or maybe 1.23456, or an error message, thrown exception, but not memory exhaustion.

This is easily solved by validation the $decimal parameter, so not high priority.

PHP Version

8.4.0, 8.3.14, 8.2.26, 8.1.31

Operating System

Windows

@iluuu1994
Copy link
Member

Some limit is reasonable, given that floating point numbers don't have infinite precision anyway. I'm not in favor of returning an incorrect result though (i.e. "1"). Maybe @SakiTakamachi, @cmb69 or @bukka can comment.

@cmb69
Copy link
Member

cmb69 commented Jan 6, 2025

Hmm, according to https://3v4l.org/TTcKA, this always errored (for some reasonable memory_limit).

Anyhow, I think we had a similar issue regarding the INI setting precision not long ago, and I argued that even values of 20 or even higher won't make any sense; the float to string conversion would just make something up.

@SakiTakamachi
Copy link
Member

SakiTakamachi commented Jan 6, 2025

I think memory exhaustion is correct in this case.

In the case of 1.23456, there will probably be a use case for $decimal where specify a number greater than 5.

e.g. 1.234560000

If limit the maximum value of $decimal, unless set it to a very small value, memory may still be exhausted depending on the environment.

Specifying a value large enough to cause memory exhaustion is a rare case, so I think there is no problem with leaving it as it is :)

edit: In practice, specifying a precision small than the given floating point number will result in an error being displayed and meaningless numbers being created, but I don't see any strong reason to prohibit this at present.

@phpfui
Copy link
Author

phpfui commented Jan 6, 2025

I think checking an argument for a valid range is a good idea. There is no reason to have a preventable error that causes a system failure. I would recommend the parameter be validated and changed to a reasonable value between 0 and say 20. Something like max(min($decimal, 20), 0)

@Girgias
Copy link
Member

Girgias commented Jan 6, 2025

Yeah, I don't really see the point in having a memory exhaustion when a simple range check and throw a ValueError is an option we have.

The range should be in sync for the precision INI setting.

@mvorisek
Copy link
Contributor

mvorisek commented Jan 6, 2025

The float range is 2.2250738585072014e-308 - 1.7976931348623158e+308

https://3v4l.org/9XZVH

so 2nd parameter should be limited to about int<-350, 350>.

@cmb69
Copy link
Member

cmb69 commented Jan 6, 2025

Off the top of my head, doubles (i.e. IEEE 754 binary 64bit values) have a mantissa of 53bits. That precision is less than 64bit long values. Makes no sense to pretend otherwise.

@SakiTakamachi
Copy link
Member

I don't have a strong opinion, so I'm not against using a valid range.

However, since $decimal can also specify negative values, we need to consider the range for negative values ​​as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants