-
Notifications
You must be signed in to change notification settings - Fork 192
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
isNumber() bugfix #262
isNumber() bugfix #262
Conversation
..anyone here? :) |
@@ -524,7 +528,10 @@ public static function isNumber($input, array $options = []) | |||
|
|||
$regexs = Zend_Locale_Format::_getRegexForType('decimalnumber', $options); | |||
$regexs = array_merge($regexs, Zend_Locale_Format::_getRegexForType('scientificnumber', $options)); | |||
if (!empty($input) && ($input[0] == $symbols['decimal'])) { | |||
|
|||
$firstChar = substr($input,0,1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this better than $input[0]
?
provide simple reproducible test case. i.e php snippet. I believe this is locale aware problem, so include your locale setup details. |
Any localization using ',' instead of '.' as decimal separator triggers the warning, when used as filter in form field definition. (As far as I remember, because I applied this fix like year+ ago. Now just patching the upstream as well. ) |
i've found a fix for localization aware float to use sprintf %F formatter |
you say "triggers the warning", but you have not provided what the warning is. and still, if you want this to be merged, provide a code snippet to reproduce it. other maintainers may be more forgiving and merge without any tests, but not me. |
Msg was something about using float as array key is not allowed. I don't remember where exactly I found the problem, so can't really reproduce immediately. Need to run into that again. Trying to get UTs working right now to verify the change. Still some work to be done there, commented in another thread (UT9 migration). |
I managed to find place of my code where "old" code is triggering the warning:
Now I'm trying to reproduce this problem using some real-life sample code to move this PR forward. |
OK, here is how to reproduce the problem / how to trigger the warning:
As you can see, array access to the float does not really work. My patch is fixing the problem. Thanks in advance. |
I have slightly updated the PR to make UTs happy. Now FloatTest is passing as well, so pls consider to merge, thanks. |
Anyone here or already dead? |
Using isNumber() in current implementation is causing float to be used as array key and that's prohibited by newer PHP versions.
Here is the bugfix for the problem. Already tested localy on production servers.