Skip to content
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.

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

Symfony\Component\Yaml\Inline::evaluateScalar() breaks ISO 8601 dates #6275

Closed
bartfeenstra opened this issue Dec 12, 2012 · 5 comments
Closed
Labels

Comments

@bartfeenstra
Copy link

Symfony\Component\Yaml\Inline::evaluateScalar() uses Symfony\Component\Yaml\Inline::getTimestampRegex() to determine whether a scalar value contains a date in what seems to be the ISO 8601 format (getTimestampRegex() is badly documented) and if it does, it converts it to a Unix timestamp.
The problem is that this conversion is conceptually impossible for a number of reasons:

  • ISO 8601 dates and Unix timestamps use different formats. Converting one to the other may break systems that expect a particular format.
  • Unix timestamps do not support dates before 1970, while ISO 8601 does. This means data loss.

Suggested solution: remove the feature entirely.

@stof
Copy link
Member

stof commented Dec 12, 2012

The feature should not be removed as handling ISO 8601 values a dates is part of the YAML spec. If you want to keep the string, you have to quote it.

@bartfeenstra
Copy link
Author

I might not be a YAML expert, but how is converting a value that can or cannot (e.g. dates before 1970) be converted to a Unix timestamp to a Unix timestamp part of the specifications? I did a quick search of the 1.0, 1.1 and 1.2 documentation and couldn't find anything about special handling for ISO 8601 or Unix timestamps.

@asm89
Copy link
Contributor

asm89 commented Dec 12, 2012

@bartfeenstra Dates before 1970 are represented by negative numbers. See the PR I created with a test showing that dates before 1970 work.

Are you having issues with this?

fabpot added a commit that referenced this issue Dec 12, 2012
This PR was merged into the master branch.

Commits
-------

2f07966 [Yaml] Add test showing that dates before 01 Jan 1970 are correctly parsed

Discussion
----------

[Yaml] Add test showing that dates before 01 Jan 1970 are correctly parsed

Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes

Adds a test related to issue #6275, showing that the Yaml inline parsing supports ISO 8601 dates before 1980.
@bartfeenstra
Copy link
Author

It appears that YYYY-MM-DD formats need to be quoted so Yaml doesn't convert them. YYYY-MM formats can be unquoted. My problems occured after manually altering some configuration files and not quoting YYYY-MM-DDD formats. A classic example of pebkac.

Thank you, stof and asm89, for your time spent on this issue!

Now that my issues are solved, I still have a few questions though:

  • Unix timestamps in PHP, which are essentially 32bits signed integers, have a very limited minimum value. -2147483647 (minimum value) / 86400 (number of seconds in a day) / 365 (days in a year) = roughly 68 years before 1970, so when converting any ISO8601 date before roughly 1902 on a 32bit system, this should result in an overflow. On 64bit systems the range of a Unix timestamp exceeds that of the default ISO 8601 format and this problem does not occur.
  • Why does Yaml convert some date formats (Symfony\Component\Yaml\Inline::getTimestampRegex() says "Unix timestamp", which is incorrect, not does it really describe what the pattern should match), but not all? To be more specific: why does it convert YYYY-MM-DD, but not YYYY-MM?

@stof
Copy link
Member

stof commented Dec 12, 2012

@bartfeenstra because of the format specified in the YAML spec for timestamps: http://www.yaml.org/spec/1.2/spec.html#id2761573

@fabpot fabpot closed this as completed in 04d95a5 Dec 15, 2012
fabpot added a commit that referenced this issue Feb 18, 2016
…buh)

This PR was merged into the 3.1-dev branch.

Discussion
----------

[Yaml] support to parse and dump DateTime objects

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #6275, #8580, #11838, #14420
| License       | MIT
| Doc PR        | TODO

Commits
-------

7e1c6c4 [Yaml] support to parse and dump DateTime objects
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants