search

ruby / psych

A libyaml wrapper for Ruby
MIT License
566 stars 205 forks source link

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

Open KJTsanaktsidis opened 5 months ago

KJTsanaktsidis commented 5 months ago

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?