Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

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

Closed
grizzm0 opened this Issue · 9 comments

5 participants

@grizzm0

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.

@grizzm0

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

@spiffyjr

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.

@grizzm0

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...

@davidwindell

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

@weierophinney

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?

@davidwindell

See also #4592

@grizzm0

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.

@davidwindell

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

@mpalourdio

after fighting with this too today, see #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
Something went wrong with that request. Please try again.