DateTime parsed invalid date

[michaljusiega]

Q	A
Bug report?	probably yes
Feature request?	no
BC Break report?	probably yes
RFC?	yes if accepted this issue

Steps required to reproduce the problem

1. @Serializer\Type("DateTime<'Y-m-d', 'Europe/Warsaw', '|Y-m-d'>")
2. Pass to value a 2020-13-03
3. Get a 2021-01-03 object DateTime

Expected Result

• I think should throws RuntimeException

Actual Result

• Valid object

Solution

In DataHandler after:

        if (false === $datetime) {
            throw new RuntimeException(sprintf('Invalid datetime "%s", expected format %s.', $data, $format));
        }

should be next check about last errors:

        if (false === $datetime) {
            throw new RuntimeException(sprintf('Invalid datetime "%s", expected format %s.', $data, $format));
        }

        if ((\DateTime::getLastErrors()['warning_count'] ?? 0) > 0) {
            throw new RuntimeException(sprintf('The parsed date "%s" was invalid', $data));
        }

[goetas]

This is not a bug but the normal PHP'd DateTime behavior. See https://3v4l.org/pHq23

if you need to validate your incoming data, you should use the preDeserialize event.

[goetas]

if ((\DateTime::getLastErrors()['warning_count'] ?? 0) > 0) {
    throw new RuntimeException(sprintf('The parsed date "%s" was invalid', $data));
}

This sounds a good thing but would result in a BC break. I can accept this new behavior only if the PR allows to opt-in for this stricter validation. (maybe passing some parameter in the constructor?

[michaljusiega]

Hi @goetas.
Thanks for answer on my issue. I know that is a expected behavior in PHP but not in the package. If I deserialize something like Date in my opinion should be one to one same like input data.

btw, PR should be on 4.0 version, right ?

[goetas]

If we make the DateHandler constructor as:

__construct(
  string $defaultFormat = \DateTime::ATOM, 
  string $defaultTimezone = 'UTC', 
  bool $xmlCData = true
  string $dateTimeParsing = 'lax', // or 'strict'
)

and then we use $dateTimeParsing to trigger the strict parsing, I think can be included in the next feature release (3.x)

[michaljusiega]

Hi. Sorry for unactivity. I saw a 3.6 release but I can't found a changes in code.

[goetas]

Hi. Sorry for unactivity. I saw a 3.6 release but I can't found a changes in code.

which changes?

[goetas]

it seems that the tool i use to generate the changelog messed up, this ticket was closed by you, so no changes have been implemented on my end.

[goetas]

As said in #1152 (comment), i'm happy to accept a PR implementing something as suggested there.

[michaljusiega]

I understand. I closed this issue because I thought I couldn't add this option on PR cause is related to bundle too.