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

Date validation accepts invalid dates #14727

Closed
peter17 opened this issue May 22, 2015 · 19 comments
Closed

Date validation accepts invalid dates #14727

peter17 opened this issue May 22, 2015 · 19 comments

Comments

@peter17
Copy link
Contributor

peter17 commented May 22, 2015

In Symfony-2.6.7, I am using a form with type date, 'format' => 'yyyy-MM-dd' and constraint => array(new Symfony\Component\Validator\Constraints\Date).

When a user inputs a date with a five-digit year, the form is validated successfully but an incorrect date is persisted.

Example:
User inputs 20011-11-11. Persisted date is 2001-11-11.

It is probably not Symfony's fault, since PHP's DateTime('20011-11-11') indeed returns object(DateTime)#1 (3) { ["date"]=> string(26) "2001-11-11 20:01:00.000000" ["timezone_type"]=> int(3) ["timezone"]=> string(16) "Europe/Amsterdam" }. See http://3v4l.org/tCrGl for version details.

20011-11-11 should either be accepted as valid or rejected as invalid but not transformed to something else and accepted... Could Symfony validate the string format before PHP transforms it to a date?

Best regards

@Maxooo
Copy link

Maxooo commented May 22, 2015

Same problem here with the 'widget' => 'single_text' option.

@Koalabaerchen
Copy link

scratches head

Could not repoduce it in 3.0.3, 2.8.3, 2.7.10, 2.6.13, 2.3.38

Can confirm that 2.6.7 behaves weird.

DateTime {#353 ▼
  +"date": "20011-11-11 00:00:00.000000"
  +"timezone_type": 3
  +"timezone": "Europe/Berlin"
}

I have no idea how that is even possible.
But "current" 2.6 version (2.6.13) throws a validation error as expected.

@xabbuh
Copy link
Member

xabbuh commented Mar 13, 2016

Closing as the problem seems to be fixed.

@peter17 @Maxooo Feel free to comment with some steps to reproduce your problem if the issue still persists for you and we can reopen this.

@xabbuh xabbuh closed this as completed Mar 13, 2016
@peter17
Copy link
Contributor Author

peter17 commented Mar 14, 2016

I still have this problem with Symfony-2.8.3.

Here is the code in my Form Type:

$builder->add('date_end', 'date', [
              'label' => 'form.label.date_end',
              'widget' => 'single_text',
              'format' => 'dd/MM/yyyy',
              'invalid_message' => 'form.error.date.format',
              'constraints' => [
                  new NotBlank([
                      'message' => 'form.error.not_blank',
                  ]),
                  new Date(),
                  new GreaterThanOrEqual([
                      'value' => new \DateTime(date('Y-m-d')),
                      'message' => 'form.error.date.after_today',
                  ]),
              ],
              'attr' => [
                  'placeholder' => 'form.label.date_format_sample',
              ],
          ])

What I type in the field is: 21/03/20107.

When I use var_dump($myObject->getDateEnd()), I get:

object(DateTime)[3934]
  public 'date' => string '2007-03-21 20:10:00.000000' (length=26)
  public 'timezone_type' => int 3
  public 'timezone' => string 'Europe/Paris' (length=12)

What is persisted in the MySQL table is: 2007-03-21.

In my opinion, Symfony's Date validator should reject 21/03/20107 since it will not give a valid PHP date (and also it does not match the format pattern).

@jakzal jakzal reopened this Mar 14, 2016
@jakzal
Copy link
Contributor

jakzal commented Mar 14, 2016

@peter17 I just tried your example and it works fine for me (I get a validation error). Could you please fork symfony standard edition and reproduce your issue on a new branch?

@peter17
Copy link
Contributor Author

peter17 commented Mar 14, 2016

@jakzal sorry, I'm not totally sure about what you expect from me: should I

  • create a new Symfony project with just a controller and a form type?
  • or try to create a new failing test in Symfony's unit tests?

When I add 20107-03-21 to the getValidDates or the getInvalidDates methods in Symfony/Component/Validator/Tests/Constraints/DateValidatorTest.php and run the unit tests, I indeed get a validation error (INVALID_FORMAT_ERROR) in the 3.0 and 2.8 branches as well as the v2.8.3 tag.

However, in my project, I used var_dump($value, $constraint) in Symfony/Component/Validator/Constraints/DateValidator.php and I noticed that the $value it receives after form submission is actually an instance of DateTime and hence the validate method returns without trying to apply the pattern......

This seems inconsistent with the documentation which specifies that:

Date validates that a value is a valid date, meaning either a DateTime object or a string [...] that follows a valid YYYY-MM-DD format.

It seems that the DateValidator never invalidates DateTime objects...


What I think I understand:

  • when the DateValidator receives a string like 20107-03-21, it rejects it (seems correct)
  • when the DateValidator receives any DateTime object, it accepts it (ok, why not)
  • when I submit a form, the input string is first converted to a DateTime object (I don't know which code does that) and then passed to the DateValidator which accepts it
  • the problem, I think, is that PHP converts "invalid" strings to a valid DateTime, like here: https://3v4l.org/Rf7XP so an "invalid" input is finally accepted by the DateValidator...

Note: adding 'input' => 'string' in my form type partially solves the problem: I indeed get a validation error but also an exception (Unable to transform value for property path "date_end": Expected a string.)...

@jakzal
Copy link
Contributor

jakzal commented Mar 14, 2016

create a new Symfony project with just a controller and a form type?

Yes, this will do. I tried your example and it works fine for me. This let me to think there might be something about you configured the project that exposes this problem.

@peter17
Copy link
Contributor Author

peter17 commented Mar 14, 2016

@jakzal I created a new minimal Symfony project here: https://github.com/peter17/DateTypeTest

Here are the results of the tests I made:
test1


test2


test3


In the 3rd case, I think I should have a validation failure...

@jakzal
Copy link
Contributor

jakzal commented Mar 14, 2016

I don't have this issue on my OS X running PHP 5.5.30. However, I just tested this on PHP 5.5.30 and 5.6.14 on Linux and confirmed the invalid behaviour. It seems to depend on the platform.

@peter17
Copy link
Contributor Author

peter17 commented Mar 14, 2016

I forgot to mention that I use PHP-5.5.29 on Linux.

@jakzal
Copy link
Contributor

jakzal commented Mar 14, 2016

It just occurred to me, should we really treat 20107 as an invalid year?

@peter17
Copy link
Contributor Author

peter17 commented Mar 14, 2016

It could be valid or invalid, or up to the users...

The problem here, as I explain above, is that when the date is a DateTime object, the DateValidator does not check it at all (on Linux, seems different on OS X...)...

@xabbuh
Copy link
Member

xabbuh commented Mar 14, 2016

From what do you deduce that the DateValidator isn't called at all? Did you confirm this by using a debugger or something like that?

@peter17
Copy link
Contributor Author

peter17 commented Mar 15, 2016

Please read what I wrote above:

I used var_dump($value, $constraint) in Symfony/Component/Validator/Constraints/DateValidator.php and I noticed that the $value it receives after form submission is actually an instance of DateTime and hence the validate method returns without trying to apply the pattern.

So, yes, it is called, but since its argument is a DateTime object, it just returns without trying to validate that argument.

@xabbuh
Copy link
Member

xabbuh commented Mar 15, 2016

Yeah, I read that. But this means that the validator actually is not skipped. It just doesn't do anything with the given value as this already is a DateTime object which seems pretty straightforward to me (PHP would complain about invalid dates, wouldn't it?). The actual transformation happens in the DateTimeToStringTransformer which simply uses built-in PHP tools (namely \DateTime::createFromFormat()).

As @jakzal said why should 20107 be treated as an invalid year? Imho if you want people to only add dates from a particular range, you should add additional constraints that check if the given dates is in this desired interval.

@peter17
Copy link
Contributor Author

peter17 commented Mar 15, 2016

I am not discussing whether 20107 should be treated as a valid year or not.

What we observed is that on some platforms, there is a validation error on those dates and on some others, none. Also, on some platforms, the dates with years > 9999 are accepted and DateTime object does not have the correct date (20107 becomes 2007).

Now, I am trying to reproduce the latter on a minimal project.

@xabbuh
Copy link
Member

xabbuh commented Dec 15, 2017

This one is tricky as IMO PHP is the part that is acting wrong here. We would need to find a way to discover whether the parsed year is the same as the one given in the original string.

@curry684
Copy link
Contributor

It's definitely Symfony's fault, but not in the Validator, nor in the specific class @xabbuh pointed at - it's in DateTimeToLocalizedStringTransformer instead, which far more naively, and wrongly, tries to convert dates. I have tested a fix which happily does persist the date in 20107, now working on a backport to 2.7 for PR.

@curry684
Copy link
Contributor

curry684 commented Jan 13, 2018

Scrap that, I had it working in a form, but the unit tests blow up all over the place because PHP's support for 5+ digit years is just too inconsistent across all functions to rely on:

$ php -r "echo strtotime('9999-12-12 12:34:56 UTC').PHP_EOL;"
253400618096
$ php -r "echo strtotime('2017-12-12 12:34:56 UTC').PHP_EOL;"
1513082096
$ php -r "echo strtotime('10000-12-12 12:34:56 UTC').PHP_EOL;"

$ php -r "echo (new \DateTime('10000-12-12 12:34:56 UTC'))->format('c').PHP_EOL;"
PHP Fatal error:  Uncaught Exception: DateTime::__construct(): Failed to parse time string (10000-12-12 12:34:56 UTC) at position 12 (1): Double time specification in Command line code:1
$ php -r "echo (\DateTime::createFromFormat('10000-12-12 12:34:56 UTC', 'Y-m-d'))->format('c').PHP_EOL;"
PHP Fatal error:  Uncaught Error: Call to a member function format() on boolean in Command line code:1
$ php -r "echo (new \DateTime(gmdate('Y-m-d', 572355244800)))->format('c').PHP_EOL;"
2007-03-21T20:10:00+01:00
$ php7.0 -r "echo (new \DateTime('20107-03-03'))->format('c').PHP_EOL;"
2007-03-03T20:10:00+01:00
$ php -r "echo gmdate('Y-m-d', 572355244800).PHP_EOL;"
20107-03-21

I'll just make it throw instead, at least then it's consistent.

fabpot added a commit that referenced this issue Jan 17, 2018
…y684)

This PR was squashed before being merged into the 2.7 branch (closes #25781).

Discussion
----------

[Form] Disallow transform dates beyond the year 9999

Fixes #14727

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | not really
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #14727
| License       | MIT

Explicitly locked out submission of dates beyond December 31st 9999 in forms as PHP is highly incapable of consistently handling such dates. Before this patch dates were randomly transformed or mangled.

Technically there is a BC break as this will now cause validation to fail on input that was *accepted* before, but it was mangled. Not my call but I prefer the rejection over data corruption:
```
// Old behavior
$transformer = new DateTimeToLocalizedStringTransformer('UTC', 'UTC', null, null, \IntlDateFormatter::GREGORIAN, 'yyyy-MM-dd');
$result = $transformer->reverseTransform('20107-03-21');

// $result is now 2007-03-21
```

Commits
-------

70cc969 [Form] Disallow transform dates beyond the year 9999
@fabpot fabpot closed this as completed Jan 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants