add try/catch around statements that execute intl_is_failure() #6598

gkralik · 2014-08-26T14:38:39Z

Since PHP 5.5.0, the configuration option 'intl.use_exceptions' is available.
When set to true, IntlException's are thrown instead of just setting a error
code, so no calls to intl_is_failure() are necessary.
Without the try/catch, the Int, Float and DateTime I18n validators hard-fail
in case 'intl.use_exceptions' is set to true.

NOTE: catching Exception (super class of IntlException), as IntlException is
not available prior to PHP 5.5.0.

refs #6312

mwillbanks · 2014-08-27T13:18:29Z

You should not just catch the entire exception. I'll try and add some additional notes later. You should be able to detect the version and wrap it properly. You may want to create a new method for the call as to properly wrap it for existing behavior.

ThaDafinser · 2014-09-25T08:19:31Z

@gkralik @mwillbanks there is the IntlException http://php.net/manual/en/class.intlexception.php

But it's only available PHP 5.5 +

ThaDafinser · 2014-09-25T08:24:45Z

@gkralik @mwillbanks what about this version:
https://github.com/ThaDafinser/zf2/blob/patch-13/library/Zend/I18n/Validator/Float.php#L122-L135

ThaDafinser · 2014-09-25T08:27:03Z

Other idea (should return no warning and proper handling later extending exceptions...)

if(class_exists('IntlException') && $ex instanceof IntlException)){
    //
}

gkralik · 2014-09-25T08:30:35Z

What's the benefit? We're throwing InvalidArgumentException anyways, so why care about whether to catch Exception or IntlException here?

gkralik · 2014-09-25T08:31:36Z

https://github.com/ThaDafinser/zf2/blob/patch-13/library/Zend/I18n/Validator/Float.php#L130 won't work for me. get_class() expects an object parameter.

ThaDafinser · 2014-09-25T08:33:37Z

@gkralik updated it...was just freely coded
https://github.com/ThaDafinser/zf2/blob/patch-13/library/Zend/I18n/Validator/Float.php#L122-L135

gkralik · 2014-09-25T08:34:50Z

@ThaDafinser OK, i like that. This way no exceptions get lost if they're not IntlException. Want me to update the PR?

ThaDafinser · 2014-09-25T08:37:25Z

@gkralik please wait for additional feedback. To be honest i dont know if this is enough, since the old parse is gone? Have to get into again
https://github.com/zendframework/zf2/blob/release-2.3.1/library/Zend/I18n/Validator/Float.php#L112-L122

ThaDafinser · 2014-09-25T08:38:57Z

Also the constanct NOT_FLOAT is not used anymore?
https://github.com/zendframework/zf2/blob/master/library/Zend/I18n/Validator/Float.php#L25

gkralik · 2014-09-25T08:40:22Z

you're right, seems to have changed quite a lot...

gkralik · 2014-09-25T08:43:25Z

OK, just checked that: Handling of IntlException is still necessary, see https://github.com/zendframework/zf2/blob/master/library/Zend/I18n/Validator/Float.php#L125

Int and DateTime validators are still the same and need exception handling...

ThaDafinser · 2014-09-25T08:45:08Z

I think @moderndeveloperllc can help, since he rewrote it
#6373

moderndeveloperllc · 2014-09-25T13:45:15Z

Someone rang? Yes, I rewrote the beast called Float because it didn't actually validate everything it should have. I should get around to Int some day when I'm feeling particularly masochistic. I probably just forgot to remove the unused constant as they were both basically the same thing. Adding try/catch seems fine with me as long as it doesn't break older versions. It doesn't mess with the actual validation logic.

Then again I'm on a rather powerful (prescribed thank you very much) painkiller at the moment, so pretty much anything sounds just fine right now :-)

gkralik · 2014-09-25T16:15:09Z

I have rebased the PR and added the check for non-IntlException.

gkralik · 2014-09-25T16:32:52Z

@moderndeveloperllc Actually, shouldn't we set the NOT_FLOAT error before returning false in lines 173 and 248 in Zend\I18n\Validator\Float?

malukenho · 2014-10-10T18:06:38Z

library/Zend/I18n/Validator/Float.php

+                throw new Exception\InvalidArgumentException($formatter->getErrorMessage());
+            }
+        } catch(\Exception $e) {
+            if(get_class($e) != 'IntlException'){


can you use instanceof ?

No. IntlException is not available prior to PHP 5.5

That is actually no problem, instanceof also works when the class doesn't exist.

@DASPRiD really!
http://3v4l.org/6VNuH

OK, didn't know that. Will change it.

malukenho · 2014-10-10T18:08:14Z

@gkralik This PR need to be rebased.

Since PHP 5.5.0, the configuration option 'intl.use_exceptions' is available. When set to true, IntlException's are thrown instead of just setting a error code, so no calls to intl_is_failure() are necessary. Without the try/catch, the Int, Float and DateTime I18n validators hard-fail in case 'intl.use_exceptions' is set to true. NOTE: catching Exception (super class of IntlException), as IntlException is not available prior to PHP 5.5.0. refs #6312

gkralik · 2014-10-14T10:29:51Z

@malukenho done.

instanceof even works for PHP < 5.5 where IntlException does not exist, so use it instead of get_class().

malukenho · 2014-10-14T18:14:18Z

library/Zend/I18n/Validator/DateTime.php

+            if (intl_is_failure($formatter->getErrorCode())) {
+                throw new ValidatorException\InvalidArgumentException($formatter->getErrorMessage());
+            }
+        } catch(\Exception $e) {


@gkralik well, I think you can't use pokémon exception handler here.
What you think about use one more catch statement?

<?php try { // ... } catch (IntlException $e) { } catch (Exception $e) { }

Good hint: only catch a specific exception type.

After looking over this again, I agree. Best will be to just catch IntlException here and ignore Exception completely. It seems PHP versions < 5.5 will not choke on this.

Ocramius · 2014-12-17T13:44:00Z

Will need to write tests for this one before merging.

Close #6598 Close #6312 Forward port #6598 Forward port #6312

Ocramius · 2015-01-03T17:36:24Z

Merged, thanks @gkralik! (I added the caught IntlExceptions as $previousException when re-thrown)

master: 19b8cba
develop: 490e5c8

…ogic' Close #7168

…ogic' into develop Close #7168 Forward port #7168

…nces as previous exceptions

…ptions' Close zendframework/zendframework#6598 Close zendframework/zendframework#6312

…ptions' into develop Close zendframework/zendframework#6598 Close zendframework/zendframework#6312 Forward port zendframework/zendframework#6598 Forward port zendframework/zendframework#6312

EvanDotPro force-pushed the master branch from f77f90b to 21a17e5 Compare September 28, 2014 00:34

malukenho reviewed Oct 10, 2014
View reviewed changes

Gregor Kralik added 2 commits October 14, 2014 09:08

rethrow exception if not IntlException

9c78601

Gregor Kralik added 2 commits October 14, 2014 16:42

use instanceof to check for IntlException

1be015b

instanceof even works for PHP < 5.5 where IntlException does not exist, so use it instead of get_class().

fix the wrong check introduced in 1be015b

f4e2ab4

malukenho reviewed Oct 14, 2014
View reviewed changes

only catch IntlException

a63b4a9

Ocramius added I18n Validator enhancement bug labels Dec 17, 2014

Ocramius removed the enhancement label Dec 17, 2014

Ocramius added this to the 2.4.0 milestone Dec 17, 2014

Ocramius self-assigned this Dec 17, 2014

Ocramius modified the milestones: 2.3.4, 2.4.0 Jan 3, 2015

Ocramius added a commit that referenced this pull request Jan 3, 2015

#6598 - CS - space after catch keyword

6704893

Ocramius added a commit that referenced this pull request Jan 3, 2015

#6598 - using caught IntlException instances as previous exceptions

b2e9d8d

Ocramius added a commit that referenced this pull request Jan 3, 2015

Merge branch 'hotfix/#6598-catch-intl-exceptions' into develop

490e5c8

Close #6598 Close #6312 Forward port #6598 Forward port #6312

Ocramius closed this in 19b8cba Jan 3, 2015

Ocramius mentioned this pull request Jan 3, 2015

[Zend\I18n] php.ini setting "intl.use_exceptions" on true #6312

Closed

Ocramius added a commit that referenced this pull request Feb 2, 2015

Merge branch 'hotfix/#7168-#6598-fix-typo-in-intl-exception-rethrow-l…

f6a01fc

…ogic' Close #7168

Ocramius added a commit that referenced this pull request Feb 2, 2015

Merge branch 'hotfix/#7168-#6598-fix-typo-in-intl-exception-rethrow-l…

56bb6fe

…ogic' into develop Close #7168 Forward port #7168

gianarb pushed a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015

zendframework/zendframework#6598 - CS - space after catch keyword

b78aea5

gianarb pushed a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015

zendframework/zendframework#6598 - using caught IntlException insta…

8381517

…nces as previous exceptions

gianarb pushed a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015

Merge branch 'hotfix/zendframework/zendframework#6598-catch-intl-exce…

ba26b1d

…ptions' Close zendframework/zendframework#6598 Close zendframework/zendframework#6312

froschdesign mentioned this pull request Feb 17, 2019

IsInt gives Warning: NumberFormatter::parse() zendframework/zend-i18n#10

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add try/catch around statements that execute intl_is_failure() #6598

add try/catch around statements that execute intl_is_failure() #6598

gkralik commented Aug 26, 2014

mwillbanks commented Aug 27, 2014

ThaDafinser commented Sep 25, 2014

ThaDafinser commented Sep 25, 2014

ThaDafinser commented Sep 25, 2014

gkralik commented Sep 25, 2014

gkralik commented Sep 25, 2014

ThaDafinser commented Sep 25, 2014

gkralik commented Sep 25, 2014

ThaDafinser commented Sep 25, 2014

ThaDafinser commented Sep 25, 2014

gkralik commented Sep 25, 2014

gkralik commented Sep 25, 2014

ThaDafinser commented Sep 25, 2014

moderndeveloperllc commented Sep 25, 2014

gkralik commented Sep 25, 2014

gkralik commented Sep 25, 2014

malukenho Oct 10, 2014

gkralik Oct 13, 2014

DASPRiD Oct 14, 2014

malukenho Oct 14, 2014

gkralik Oct 14, 2014

malukenho commented Oct 10, 2014

gkralik commented Oct 14, 2014

malukenho Oct 14, 2014

Ocramius Oct 14, 2014

gkralik Oct 15, 2014

Ocramius commented Dec 17, 2014

Ocramius commented Jan 3, 2015

add try/catch around statements that execute intl_is_failure() #6598

add try/catch around statements that execute intl_is_failure() #6598

Conversation

gkralik commented Aug 26, 2014

mwillbanks commented Aug 27, 2014

ThaDafinser commented Sep 25, 2014

ThaDafinser commented Sep 25, 2014

ThaDafinser commented Sep 25, 2014

gkralik commented Sep 25, 2014

gkralik commented Sep 25, 2014

ThaDafinser commented Sep 25, 2014

gkralik commented Sep 25, 2014

ThaDafinser commented Sep 25, 2014

ThaDafinser commented Sep 25, 2014

gkralik commented Sep 25, 2014

gkralik commented Sep 25, 2014

ThaDafinser commented Sep 25, 2014

moderndeveloperllc commented Sep 25, 2014

gkralik commented Sep 25, 2014

gkralik commented Sep 25, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

malukenho commented Oct 10, 2014

gkralik commented Oct 14, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ocramius commented Dec 17, 2014

Ocramius commented Jan 3, 2015