Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Zend\Form\Element\DateTime broken due to filter #4889

Closed
grizzm0 opened this Issue Jul 26, 2013 · 9 comments

Comments

Projects
None yet
5 participants
Contributor

grizzm0 commented Jul 26, 2013

Zend\Form\Element\DateTime is broken as of #3632 due to added Zend\Filter\DateFormatter.

If you enter "0.1" as value the filter will return a DateTime object with todays date and the time 01:00:00. The validator will then grab the DateTime object rather than the real value (as validators work with filtered value) and try to validate against that. And a DateTime object is always considered as valid.

Basically, the validator is never "really" executed. And any format of the string is valid as long as the filter can turn it into a DateTime object.

Contributor

grizzm0 commented Jul 26, 2013

My suggestion is either to remove the filter as a default from DateTime element or try to fix the filter using createFromFormat. As it is now the filter will take any string and try to convert it into a DateTime object. Which either results in a DateTime object or an exception. The filter was originally introduced to fix different browsers sending different formats, so I guess this is a tough one.

Pinig @davidwindell

Contributor

spiffyjr commented Jul 26, 2013

Removing the filter will cause the element to no longer return a DateTime object which will break BC. I agree that the filter should just be removed and leave it to hydrators to manage type conversions but we can't do that without breaking BC. The DateFormatter filter needs to be updated to reject invalid strings.

Contributor

grizzm0 commented Jul 29, 2013

Just realized that the filter doesn't return a DateTime object. No idea how I missed that the first time... It returns $dateTime->format($format) which is a string...

Contributor

davidwindell commented Jul 29, 2013

We should probably just remove the validators on these elements that use the DateTimeFormatter.

Owner

weierophinney commented Aug 1, 2013

I like the suggestion from @grizzm0 to fix the filter to use createFromFormat(). However, this poses two problems: (1) filters, as a general rule, should not raise exceptions when filtering, and (2), as @grizzm0 notes, the filter don't actually return DateTime objects currently.

Removing the validators on the elements, per @davidwindell, is a possibility, but then that means that validation has to be added by users. That said, that makes a lot of sense to me -- the developer should know what date ranges are valid for the domain, and those ranges will vary based on the application. (E.g., entering a birth date will invariably be a date in the past, while a reservation would be a date in the future.)

I'd recommend that approach; who wants to do the work?

Contributor

davidwindell commented Aug 2, 2013

See also #4592

Contributor

grizzm0 commented Aug 7, 2013

The createFromFormat() idea from me doesn't make much sense tho when I gave it some thought. It expects a format and returns the same format. It wont really do anything... There's nothing wrong with the validator. I'd say remove the filter on the DateTime element and keep the validator.

Also, as @weierophinney said, the filter should not raise an exception. As it is now the user can cause a 500 application error as long as \DateTime can't create a date from the given string.

Contributor

davidwindell commented Aug 7, 2013

I agree with filters not throwing exceptions, but I was told otherwise see the outdated discussion at zendframework#3632 (diff)

Contributor

mpalourdio commented Oct 24, 2013

after fighting with this too today, see zendframework#5340 , I agree that removing the filter could be a good solution. Anyway, it returned a string, and a validator checks if the format is correct.

If the options=>array('format'=>'d/m/Y') is provided by the developper, the validator will still be fired. So we could simply remove the filter in getinputSpecification();

Of course it will be BC break for those who use date filtering on form submission, but TBH actually, it causes much headaches than it solves things when dealing with multiple date formats

Filtering date should be added manually, according the situation the developper is. Or a relevant filter would be a conversion string to DateTime, but it implies playing with regexp and so for format conversions, separators etc.. Can be very tricky.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment