Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Validator] Validate timezone before passing to IntlDateFormatter#33902
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
nicolas-grekas left a comment
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.
(with one minor comment about perf)
PandaaAgency commentedDec 2, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
any news on this ? since my 4.3.5 update i have the same error while validator datetime string with |
fancyweb left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
@nicolas-grekas Did you remove your comment about perf?
My bad I just found it in the hidden conversation
nicolas-grekas commentedDec 4, 2019
My perf comment is here, and is not addressed for now: |
fancyweb commentedDec 9, 2019
@sjdaws Do you mind if I finish this PR? |
sjdaws commentedDec 9, 2019
@fancyweb sure, I just need it fixed so I can update 4.3. I’m not sure what else I need to do for this PR |
nicolas-grekas commentedDec 13, 2019
…zones (fancyweb)This PR was merged into the 3.4 branch.Discussion----------[Validator][ConstraintValidator] Safe fail on invalid timezonesCo-authored-by: Scott Dawson <scott@loyaltycorp.com.au>| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | yes| New feature? | no| Deprecations? | no| Tickets |#33901| License | MIT| Doc PR |Alternative to#33902.I will explain why I think it is better this way:1. We set the timezone with the setter because it's 100% safe, it never fails. It fall backs to the default timezone if the provided timezone is not supported (as if we passed null, so the same behavior that always existed). We are therefore compatible with all edge cases.2. We don't validate the timezone with `\DateTimeZone::listIdentifiers()`. It only returns full identifiers like "Europe/Paris" but it doesn't take into account "numeric" identifiers such as "+08:00" which are perfectly valid. I added a test case to ensure we stay valid with this case. + some invalid identifiers for the native `\IntlDateFormatter` are valid with the polyfill that uses `\DateTimeZone` (eg : `X`). I don't think we can validate anything safely that will work reliably on both implementations.Commits-------3b1b994 [Validator][ConstraintValidator] Safe fail on invalid timezones
This change validates timezones before passing them to the IntlDateFormatter to prevent fatal errors where an invalid timezone is used. This wasn't previously passed in 4.3.4 and was added in 4.3.5 which causes a break when using valid ISO8601 zulu timestamps. If an invalid timezone is provided it will fall back to system default, which was used for all versions < 4.3.4.