8000 [Form] DateType fails parsing when midnight is not a valid time · Issue #19531 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Form] DateType fails parsing when midnight is not a valid time #19531

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.

Sign up for GitHub

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

Jump to bottom
Closed
Jean85 opened this issue Aug 4, 2016 · 5 comments
Closed

[Form] DateType fails parsing when midnight is not a valid time #19531

Jean85 opened this issue Aug 4, 2016 · 5 comments
Labels
Bug Form Status: Reviewed

Comments

@Jean85
Copy link
Contributor
Jean85 commented Aug 4, 2016

In some timezones, midnight may not be a valid date; this happens when the DST change was done on midnight. It's a poor choice, and it was done only in the past. If you enter those dates in a DateType, it gets rejected as invalid.

Under the hood, the issue is with the \IntlDateFormatter that is initialized with the current timezone, when with a simple date it can be avoided... Here: https://github.com/symfony/symfony/blob/3.1/src/Symfony/Component/Form/Extension/Core/DataTransformer/DateTimeToLocalizedStringTransformer.php#L119

Maybe decoupling between DateType and DateTimeType could be a solution?

In my case, TZ is Europe/Rome and one of the infamous date is 1978-05-28; I can reproduce the issue in this 3val: https://3v4l.org/NDnbm
DateTime doesn't suffer this bug: https://3v4l.org/ZmQ1q

Mainly, the Form uses the intl extension, that uses ICU, and that assumes midnight as the hour if not specified, which is what happens when you use a DateType field.

I tried to open a bug on https://bugs.php.net/bug.php?id=72649 but it got closed... What can we do about it?

PS: this issue surfaced already multiple times: #18267 #16518 #16368
@mbeccati
Copy link
Contributor
mbeccati commented Aug 4, 2016

For reference, I've closed the php.net bug as ICU returns an error and there's little we can do in the intl extension. Sure, ICU could be smarter, but its behaviour can't be altered.

In any case, think that the error lies in the way the date is parsed though. Since we're not talking about date+time, the date-only should be parsed using UTC, then eventually converted to a \DateTime in the local timezone, e.g.: https://3v4l.org/WivPk

@mbeccati mbeccati mentioned this issue Aug 5, 2016
Merged
@Jean85
Copy link
Contributor Author
Jean85 commented Aug 5, 2016

Thanks for the PR!!
It includes a test that reproduces the bug, we're good to go!

Status: reviewed

fabpot added a commit that referenced this issue Aug 13, 2016
@fabpot
bug #19541 Fix #19531 [Form] DateType fails parsing when midnight is …
b405df0 
…not a valid time (mbeccati)

This PR was merged into the 2.7 branch.

Discussion
----------

Fix #19531 [Form] DateType fails parsing when midnight is not a valid time

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #19531
| License       | MIT
| Doc PR        |

Commits
-------

c951bb6 Fix #19531 [Form] DateType fails parsing when midnight is not a valid time
@fabpot fabpot closed this as completed Aug 13, 2016
@mbeccati
Copy link
Contributor

Thanks @fabpot

fabpot added a commit that referenced this issue Aug 16, 2016
@fabpot
Merge branch '2.7' into 2.8
609ee2d 
* 2.7:
  [Routing] Add missing options in docblock
  [VarDumper] Fix dumping continuations
  [HttpFoundation] fixed Request::getContent() reusage bug
  [Form] Skip CSRF validation on form when POST max size is exceeded
  Enhance the phpDoc return types so IDEs can handle the configuration tree.
  fixes
  Remove 3.0 from branch suggestions for fixes in PR template
  [Process] Strengthen Windows pipe files opening (again...)
  Fix #19531 [Form] DateType fails parsing when midnight is not a valid time
fabpot added a commit that referenced this issue Aug 16, 2016
@fabpot
Merge branch '2.8' into 3.1
d7f8ca7 
* 2.8:
  [Routing] Add missing options in docblock
  [VarDumper] Fix dumping continuations
  [HttpFoundation] fixed Request::getContent() reusage bug
  [Form] Skip CSRF validation on form when POST max size is exceeded
  Enhance the phpDoc return types so IDEs can handle the configuration tree.
  fixes
  Remove 3.0 from branch suggestions for fixes in PR template
  [Process] Strengthen Windows pipe files opening (again...)
  Fix #19531 [Form] DateType fails parsing when midnight is not a valid time
nicolas-grekas added a commit that referenced this issue Aug 16, 2016
@nicolas-grekas
Merge branch '3.1'
77e0161 
* 3.1:
  [Routing] Add missing options in docblock
  [VarDumper] Fix dumping continuations
  [PropertyInfo] Fix an error in PropertyInfoCacheExtractor
  [HttpFoundation] fixed Request::getContent() reusage bug
  [Form] Skip CSRF validation on form when POST max size is exceeded
  Use try-finally where it possible
  [DependencyInjection] ContainerBuilder: Remove obsolete definitions
  Enhance the phpDoc return types so IDEs can handle the configuration tree.
  fixes
  Remove 3.0 from branch suggestions for fixes in PR template
  [Process] Strengthen Windows pipe files opening (again...)
  [Cache] Handle unserialize() failures gracefully
  Fix #19531 [Form] DateType fails parsing when midnight is not a valid time
@mvrhov mvrhov mentioned this issue Aug 18, 2016
Closed
@dkorsak
Copy link
dkorsak commented Aug 24, 2016 •
edited
Loading

After merge, there is bug on the DateTimeType::class form type - DateTimeToLocalizedStringTransformer class on branch master.

My timezone is Europe/Warsaw, +2h from UTC timezone.

Part of form:

$builder->add(
                'publishDateEnd',
                DateTimeType::class,
                [
                    'required' => false,
                    'widget' => 'single_text',
                    'format' => 'dd.MM.yyyy HH:mm',
                    'attr' => [
                        'placeholder' => 'dd.mm.yyyy HH:mm',
                    ],
                ]

When for example publishDateEnd is set as 2016-08-01 10:00:00 and I set in the web form date as 2016-08-01 12:00:00, after save form date back to 2016-08-01 10:00:00. Should be saved as 2016-08-01 12:00:00.

When I add 'view_timezone' => 'UTC' into widget option, date is set as correct date.

@jakzal
Copy link
Contributor
jakzal commented Aug 24, 2016

@dkorsak can you please open a new issue?

@dkorsak dkorsak mentioned this issue Aug 24, 2016
Closed
This was referenced Sep 2, 2016
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Form Status: Reviewed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@fabpot @javiereguiluz @jakzal @mbeccati @dkorsak @Jean85 @carsonbot
0