FriendsOfSymfony1 / symfony1

[DEPRECATED -- Use Symfony instead] Fork of symfony 1.4 with DIC, form enhancements, latest Swiftmailer, better performance, composer compatible and PHP 8 support
https://symfony.com/legacy
MIT License
338 stars 176 forks source link

sfValidatorDate throws sfValidatorError with array as $value and therefore not usable in the message #317

Open flohoss opened 8 months ago

flohoss commented 8 months ago

When using the Date Validator and providing a Date 2024-11-31 (only 30 Days in November), the Validator correctly throws a sfValidatorError but with the value as array. This happens multiple times in that file...

The exact position it happend is inside the convertDateArrayToString function where the value is an array but not yet converted to a string.

// convert array to date string
if (is_array($value))
{
  $value = $this->convertDateArrayToString($value);
}

image

When trying to use %value% in the error message it won't be replaced, because getArguments of the class correctly checks for an array and returns nothing in the end. Therefore nothing of the error message will be replaced.

image

connorhu commented 8 months ago

I think PR does not solve the problem, but bypasses the API of sfValidatorError. If you look at how the tests use sfValidatorError you will see that it uses the keys of the array passed in argument 3 when creating the message, in the form %key%. The problem here is that the value is an array that you cannot correctly use in the message. We need to find another solution to solve the problem.

flohoss commented 8 months ago

The PR would give the option to use %year% %month% and %day% in the message but not %value%. I agree, it will not solve the problem but should give an idea what the problem here is... Let's think about it and come up with a solution. Difficult because how can you use

$clean = sprintf(
    '%04d-%02d-%02d %02d:%02d:%02d',
    (int) $value['year'],
    (int) $value['month'],
    (int) $value['day'],
    0,
    0,
    0
);

without validating the values first 😕

connorhu commented 8 months ago

My idea is create a new option for the date validator: array_value_format. If it is exists and filled with value like this: %year%-%month%-%day%. Validator convert the array back to string and you can use it in message. This solution rather follows the format of other validator messages. On the other hand, your solution doesn't break anything, it just requires the key of the date array to be specified in the message instead of %value% placeholder. In any case, we need to write a test case for the example you provided and test the expected behavior.

connorhu commented 8 months ago

Btw, let's see how symfony do this? https://github.com/symfony/symfony/blob/7.1/src/Symfony/Component/Validator/Constraints/DateValidator.php#L54 and https://github.com/symfony/symfony/blob/7.1/src/Symfony/Component/Validator/ConstraintValidator.php#L73-L125

connorhu commented 8 months ago

One possible solution: https://github.com/FriendsOfSymfony1/symfony1/compare/master...connorhu:symfony1:fix/issue317 Another direction is that the format can be changed via a option. WDYT guys? // @thePanz @thirsch @alquerci

alquerci commented 8 months ago

Yes, let's go this way.

I suggest being more strict.

As, the value will be displayed to end-users. And input value is going from end-users too.

  1. To avoid the array to string conversion. a. Replace non-scalar item values with 0 or an empty string.
  2. To avoid undefined index. a. Add missing keys with 0 value
  3. Ignore extra keys like the validator do.

Hum, what is the end-user interface that use this validator ?