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

Serialisation of invalid dates is incorrect (maybe?) #672

Open
KJTsanaktsidis opened this issue Jun 24, 2024 · 1 comment
Open

Serialisation of invalid dates is incorrect (maybe?) #672

KJTsanaktsidis opened this issue Jun 24, 2024 · 1 comment

Comments

@KJTsanaktsidis
Copy link

KJTsanaktsidis commented Jun 24, 2024

Bear with me here - I'm not sure if this is a bug in Psych or I'm just misunderstanding the spec.

If you try and serialise a string that looks like a date, Psych will correctly quote it, so that it doesn't get deserialised as a date:

irb> Psych.dump('2016-09-30')
=> "--- '2016-09-30'\n"
irb> Psych.load(Psych.dump('2016-09-30')).class
=> String

However, if you try and serialise an invalid date (like september 31), then it does not quote the output:

irb> Psych.dump('2016-09-31')
=> "--- 2016-09-31\n"

This becomes a problem because it doesn't round-trip with safe_load anymore - Psych must try and instantiate '2016-09-31' as a date to realise that it's not a date and should be a string instead, but of course, it refuses to try:

irb> Psych.safe_load(Psych.dump('2016-09-31'))
Psych::DisallowedClass: Tried to load unspecified class: Date
from /var/zendesk/bundle/ruby/3.2.0/gems/psych-5.1.2/lib/psych/class_loader.rb:99:in `find'

This obviously does deserialise correctly if you allow Date though:

irb> Psych.safe_load(Psych.dump('2016-09-31'), permitted_classes: ['Date'])
=> "2016-09-31"
irb> Psych.safe_load(Psych.dump('2016-09-31'), permitted_classes: ['Date']).class
=> String

In my opinion, I think Psych.dump('2016-09-31') should return "--- '2016-09-31'\n", and not "--- 2016-09-31\n". I think https://yaml.org/type/timestamp.html is the relevant part of the YAML spec here, and it seems to define the timestamp type in terms of a regular expression, without any reference to a calendar. Since 2016-09-31 matches the regular expression, it should be quoted.

I can send a PR if we agree with the behaviour change here. WDYT?

@ulidtko
Copy link

ulidtko commented Dec 26, 2024

Please see #262

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

No branches or pull requests

2 participants