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

[5.3] validateDateFormat breaks on dates with timezones #16842

Closed
mfn opened this issue Dec 17, 2016 · 9 comments
Closed

[5.3] validateDateFormat breaks on dates with timezones #16842

mfn opened this issue Dec 17, 2016 · 9 comments

Comments

@mfn
Copy link
Contributor

mfn commented Dec 17, 2016

  • Laravel Version: 5.3.28
  • PHP Version: 7.0

Steps To Reproduce:

$ ./artisan tinker
Psy Shell v0.8.0 (PHP 7.0.14-1+deb.sury.org~trusty+1 — cli) by Justin Hileman
>>> \Illuminate\Foundation\Application::VERSION
=> "5.3.28"
>>> Validator::make(['date' => '1991-07-03T12:00:00Z'], ['date' => 'date_format:Y-m-d\TH:i:sP'])->errors()->all();
=> [
     "The date does not match the format Y-m-d\TH:i:sP.",
   ]

This broke after #16692

Before:

$ ./artisan tinker
Psy Shell v0.7.2 (PHP 7.0.14-1+deb.sury.org~trusty+1 — cli) by Justin Hileman
>>> \Illuminate\Foundation\Application::VERSION
=> "5.3.26"
>>> Validator::make(['date' => '1991-07-03T12:00:00Z'], ['date' => 'date_format:Y-m-d\TH:i:sP'])->errors()->all();
=> []
>>>
@mfn
Copy link
Contributor Author

mfn commented Dec 17, 2016

ping @rohitsubedi

@themsaid
Copy link
Member

themsaid commented Dec 17, 2016

@mfn I'd like to explain that #16692 actually fixes multiple issues with date format, so the success you had before was because date format validation was broken, not because it should work.

1991-07-03T12:00:00+02:00 is the valid value

@mfn
Copy link
Contributor Author

mfn commented Dec 17, 2016

1991-07-03T12:00:00+02:00 is the valid value

Please read https://en.wikipedia.org/wiki/ISO_8601 (the examples on the right, right at the top of the page

Combined date and time in UTC:
2016-12-17T14:40:28+00:00
2016-12-17T14:40:28Z
20161217T144028Z

Y-m-d\TH:i:sP correctly parses 1991-07-03T12:00:00Z, and 1991-07-03T12:00:00Z is a valid date time.

The PR introduced a bug.

@themsaid
Copy link
Member

@mfn you have a fix?

@mfn
Copy link
Contributor Author

mfn commented Dec 17, 2016

Better, a plan:

  • revert the PR
  • add my example as a test case
  • let the ones who want to fix their bug create a new PR without breaking the new test case

@themsaid
Copy link
Member

I don't think PHP considers it as valid, check http://php.net/manual/en/function.date.php

c ISO 8601 date (added in PHP 5)

DateTime::createFromFormat("c", "1991-07-03T12:00:00Z") returns false.

@mfn
Copy link
Contributor Author

mfn commented Dec 17, 2016

Nor does it what you would consider "the value" (tested with 7.0.14):

>>> DateTime::createFromFormat("c", "1991-07-03T12:00:00+02:00")
=> false

Probably something else going on with createFromFormat and the c modifier.

@themsaid
Copy link
Member

themsaid commented Dec 17, 2016 via email

@delmicio
Copy link

Maybe this is related to the note in PHP docs http://php.net/manual/en/class.datetime.php#datetime.constants.iso8601

Note: This format is not compatible with ISO-8601, but is left this way for backward compatibility reasons. Use DateTime::ATOM or DATE_ATOM for compatibility with ISO-8601 instead.

So you should use
DateTime::createFromFormat(DATE_ATOM, "1991-07-03T12:00:00+02:00")

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

No branches or pull requests

3 participants