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

Incompatibility with Numericality from v3.3.* to v3.4.2 #13843

Closed
mbrostami opened this issue Feb 19, 2019 · 4 comments

Comments

Projects
4 participants
@mbrostami
Copy link
Contributor

commented Feb 19, 2019

Expected and Actual Behavior

Validating string '123,123' by Numericality validator is true in Phalcon v3.3.2 but is false in v3.4.2.

Provide minimal script to reproduce the issue

$validator = new Numericality();
$validation = new Validation();
$validation->add('km', $validator);
var_dump($validation->validate(['km' => '123,123']));
die;

This is the line which is changed (!is_numeric(value) has been added in v3.4.x):
https://github.com/phalcon/cphalcon/blob/3.4.x/phalcon/validation/validator/numericality.zep#L74

@Jeckerson

This comment has been minimized.

Copy link
Member

commented Feb 28, 2019

#13450

In this issue problem is resolved with correcting setlocale(), which make sense. Because default behavior of is_numeric() is validate only float number with . and not with ,.

@niden niden added the Bug - Low label Mar 11, 2019

@niden niden added this to To do in 4.0 Release via automation Mar 11, 2019

@joeyhub

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2019

You should at all costs in PHP be avoiding anything to do with setlocale, that's for under the hood and a legacy horror that'll break a lot of things. You should never change it or rely on it. Instead use an appropriate library for intl. intl is the standard which should be used in place of the inbuilt PHP functions, however it's a bit bulky, when I last used it some message formats segfault and I'm not sure if it handles input as well as output.

PSR-joeyhub which supercedes all numeric PSRs and is the final PSR in the series almost entirely forbids the use of setlocale which must be locked to an appropriate system default (IE, C.utf8) as opposed to localised. Same as time, time must be UTC and instead DateTime used for converting to other timezones on input/output.

Take a look here:

https://www.php.net/manual/en/class.numberformatter.php

construct, format and parse are all good.

@mbrostami

This comment has been minimized.

Copy link
Contributor Author

commented Mar 31, 2019

#13450

In this issue problem is resolved with correcting setlocale(), which make sense. Because default behavior of is_numeric() is validate only float number with . and not with ,.

Agree with that, but if Phalcon follows semantic versioning, then v3.4.x should not break v3.3.x functionality. I mean it could be fixed in v4.x.x.

@niden niden referenced this issue May 17, 2019

Merged

T13843 numericality #14085

4 of 4 tasks complete

@niden niden moved this from To do to In progress in 4.0 Release May 17, 2019

@niden

This comment has been minimized.

Copy link
Member

commented May 17, 2019

Resolved in #14085

@niden niden closed this May 17, 2019

4.0 Release automation moved this from In progress to Done May 17, 2019

@niden niden added the 4.0 label Jun 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.