46 ms·

That's what I thought so I did a second take. This is the relevant excerpt: > This tree of strings is then converted to YAML tokens using the class Psych::Scal

by Yver 12y ago

That's what I thought so I did a second take. This is the relevant excerpt: > This tree of strings is then converted to YAML tokens using the class Psych::ScalarScanner that internally uses regular expressions to identify YAML types. The regular expression used to identity time objects changed in the new version of Psych, which meant our nanosecond precision time was now correctly converted to a YAML Time type rather than being left as a string. Apparently, the "hurt" mentionned in the title refers to a bug being fixed.

makomk 12y ago
It may technically be a bug fix, but the "fixed" behaviour is kind of gnarly, especially in the way that it interacts with older Ruby versions. Basically, they're serializing a string containing a timestamp as YAML, but because that string matches a regex deep in Ruby 2.1's YAML code it actually gets serialised as a YAML Time object. Ruby 1.9 then deserialises the YAML Time object to a Ruby time object rather than a string, causing round-tripping issues.
- jrochkind1 12y ago
  Yeah, this is actually pretty crazy: In ruby 1.9, a String containing a timestamp gets serialized as a String -- and parsed as a String in 1.9 or 2.1. In ruby 2.1, a String containing a timestamp gets serialized as a Time (I'm not sure I'd consider this correct behavior), -- and parsed in again as a String anyway in ruby 2.1 (due to new YAML-safety behavior), but as a Time in ruby 1.9. The ruby 2.1 behavior of serializing out as a Time but then parsing back in to a String anyway -- seems particularly sketchy, and ripe for roundtrip bugs where the string it gets turned into on parse isn't _quite_ the same one that was original before serialization. (Say, winds up with a different timezone or something). I'm not sure what the lesson here is, but I don't think it's about regexen. It may be about how too much cleverness/magic will get you -- just keep it simple. Shared gem/library code should be predictable and understandable with simple mental models, and gems used as core dependencies should be very stable from version to version (yaml and yaml serialization rules are neither). It may be, yet again, about how using YAML the way we use YAML is a big mistake -- either use ruby marshal if you actually want exact roundtrip ruby objects, or serialize only to JSON-compatible datatypes (whether using JSON or maybe that's what YAML should have done all along). This middle ground of YAML where it's hard to predict what is round trippable and what isn't, what is safe and what isn't, and it can change from version-to-version -- is just asking for trouble. But it's hard to decide to stop doing it, as so many of your gem dependencies might be.
  - tenderlove 12y ago
    > The ruby 2.1 behavior of serializing out as a Time but then parsing back in to a String anyway -- seems particularly sketchy, and ripe for roundtrip bugs where the string it gets turned into on parse isn't _quite_ the same one that was original before serialization. (Say, winds up with a different timezone or something). It's not serializing it out as a time, but serializing it as an "implicit string" (a string with no quotes). When Psych goes to dump the string out, it checks to see if the string could be interpreted as something else. Since the string being dumped doesn't match a YAML timestamp, it doesn't add explicit quoting on the string. Here's an example. From the blog post, they are doing this: Psych.dump Time.now.utc.strftime("%Y-%m-%d %H:%M:%S.%6N %Z") Output: "--- 2014-05-23 15:42:29.882127 UTC\n...\n" The `strftime` adds the string "UTC" to the timestamp. But "UTC" isn't part of the [yaml timestamp spec](http://yaml.org/type/timestamp.html http://yaml.org/type/timestamp.html). Since it doesn't match a YAML timestamp, Psych dumps it out as an implicit string. If we change the `strftime` to produce a string that does look like a YAML timestamp like this: Psych.dump Time.now.utc.strftime("%Y-%m-%d %H:%M:%S.%6N %z") Output: "--- '2014-05-23 15:46:31.197338 +0000'\n" Then the output is quoted so that when we load it isn't ambiguous (quoted strings are always considered strings, and are never candidates for coercion). If we were to manually modify that YAML and remove the quotes, it is indeed converted to a Time object: irb(main):012:0> Psych.load "--- '2014-05-23 15:46:31.197338 +0000'\n" => "2014-05-23 15:46:31.197338 +0000" irb(main):013:0> Psych.load "--- 2014-05-23 15:46:31.197338 +0000\n" => 2014-05-23 08:46:31 -0700 The article says: >The same would happen in Psych 2.0.5 if it were not for a new feature called Safe Load which was introduced after the recent Rails YAML deserialisation security vulnerabilities. I have no idea where they got this. You have to explicitly call `safe_load`, and it doesn't sound like they're doing that. If your format the timestamp correctly in your YAML, it will happily deserialize as a Time object. :-/
    - jrochkind1 12y ago
      So, the fact that there's such thing as an 'implicit string' in the YAML data model, and that I didn't know this, and that I still don't completely understand how it works even after reading your comment (thanks though!)... ...is leading me back to suggesting YAMLs data model is overly complex, and not understandable with a simple developer mental model, and that's the root of a lot of problems. I previously thought leaving quotes off string literals in YAML was simply a convenience for hand-writing YAML and producing YAML with less noise for human readability; I didn't realize it resulted in an 'implicit string' which would be de-serialized by YAML parsers differently than quoted strings. (Are these patterns by which 'implicit strings' are recognized to be coerceable to certain types part of the YAML standard, or up to the particular YAML parsing implementation as to how/whether to do it? If the latter... woah, just asking for trouble, no wonder we got it.) update Oh wait, I just realized I totally misunderstood what you were saying. Strings are Strings, but on serializing, the serializer is supposed to ensure quotes are used if the string, were it unquoted would be misinterpreted as something other than a string. Okay, seems reasonable -- but OP is reporting this is failing somehow, right? And in some cases something meant to be a string is winding up unquoted, and being interpreted as something else on load? Or I'm probably still not quite getting it. Okay, yeah, tldr, we can stick with: YAML's data/processing model ends up being way too complex making it's behavior hard to predict and hard to maintain consistent between parsers/versions.

by Yver 12y ago

makomk 12y ago

It may technically be a bug fix, but the "fixed" behaviour is kind of gnarly, especially in the way that it interacts with older Ruby versions. Basically, they're serializing a string containing a timestamp as YAML, but because that string matches a regex deep in Ruby 2.1's YAML code it actually gets serialised as a YAML Time object. Ruby 1.9 then deserialises the YAML Time object to a Ruby time object rather than a string, causing round-tripping issues.

jrochkind1 12y ago
Yeah, this is actually pretty crazy: In ruby 1.9, a String containing a timestamp gets serialized as a String -- and parsed as a String in 1.9 or 2.1. In ruby 2.1, a String containing a timestamp gets serialized as a Time (I'm not sure I'd consider this correct behavior), -- and parsed in again as a String anyway in ruby 2.1 (due to new YAML-safety behavior), but as a Time in ruby 1.9. The ruby 2.1 behavior of serializing out as a Time but then parsing back in to a String anyway -- seems particularly sketchy, and ripe for roundtrip bugs where the string it gets turned into on parse isn't _quite_ the same one that was original before serialization. (Say, winds up with a different timezone or something). I'm not sure what the lesson here is, but I don't think it's about regexen. It may be about how too much cleverness/magic will get you -- just keep it simple. Shared gem/library code should be predictable and understandable with simple mental models, and gems used as core dependencies should be very stable from version to version (yaml and yaml serialization rules are neither). It may be, yet again, about how using YAML the way we use YAML is a big mistake -- either use ruby marshal if you actually want exact roundtrip ruby objects, or serialize only to JSON-compatible datatypes (whether using JSON or maybe that's what YAML should have done all along). This middle ground of YAML where it's hard to predict what is round trippable and what isn't, what is safe and what isn't, and it can change from version-to-version -- is just asking for trouble. But it's hard to decide to stop doing it, as so many of your gem dependencies might be.
- tenderlove 12y ago
  > The ruby 2.1 behavior of serializing out as a Time but then parsing back in to a String anyway -- seems particularly sketchy, and ripe for roundtrip bugs where the string it gets turned into on parse isn't _quite_ the same one that was original before serialization. (Say, winds up with a different timezone or something). It's not serializing it out as a time, but serializing it as an "implicit string" (a string with no quotes). When Psych goes to dump the string out, it checks to see if the string could be interpreted as something else. Since the string being dumped doesn't match a YAML timestamp, it doesn't add explicit quoting on the string. Here's an example. From the blog post, they are doing this: Psych.dump Time.now.utc.strftime("%Y-%m-%d %H:%M:%S.%6N %Z") Output: "--- 2014-05-23 15:42:29.882127 UTC\n...\n" The `strftime` adds the string "UTC" to the timestamp. But "UTC" isn't part of the [yaml timestamp spec](http://yaml.org/type/timestamp.html http://yaml.org/type/timestamp.html). Since it doesn't match a YAML timestamp, Psych dumps it out as an implicit string. If we change the `strftime` to produce a string that does look like a YAML timestamp like this: Psych.dump Time.now.utc.strftime("%Y-%m-%d %H:%M:%S.%6N %z") Output: "--- '2014-05-23 15:46:31.197338 +0000'\n" Then the output is quoted so that when we load it isn't ambiguous (quoted strings are always considered strings, and are never candidates for coercion). If we were to manually modify that YAML and remove the quotes, it is indeed converted to a Time object: irb(main):012:0> Psych.load "--- '2014-05-23 15:46:31.197338 +0000'\n" => "2014-05-23 15:46:31.197338 +0000" irb(main):013:0> Psych.load "--- 2014-05-23 15:46:31.197338 +0000\n" => 2014-05-23 08:46:31 -0700 The article says: >The same would happen in Psych 2.0.5 if it were not for a new feature called Safe Load which was introduced after the recent Rails YAML deserialisation security vulnerabilities. I have no idea where they got this. You have to explicitly call `safe_load`, and it doesn't sound like they're doing that. If your format the timestamp correctly in your YAML, it will happily deserialize as a Time object. :-/
  - jrochkind1 12y ago
    So, the fact that there's such thing as an 'implicit string' in the YAML data model, and that I didn't know this, and that I still don't completely understand how it works even after reading your comment (thanks though!)... ...is leading me back to suggesting YAMLs data model is overly complex, and not understandable with a simple developer mental model, and that's the root of a lot of problems. I previously thought leaving quotes off string literals in YAML was simply a convenience for hand-writing YAML and producing YAML with less noise for human readability; I didn't realize it resulted in an 'implicit string' which would be de-serialized by YAML parsers differently than quoted strings. (Are these patterns by which 'implicit strings' are recognized to be coerceable to certain types part of the YAML standard, or up to the particular YAML parsing implementation as to how/whether to do it? If the latter... woah, just asking for trouble, no wonder we got it.) update Oh wait, I just realized I totally misunderstood what you were saying. Strings are Strings, but on serializing, the serializer is supposed to ensure quotes are used if the string, were it unquoted would be misinterpreted as something other than a string. Okay, seems reasonable -- but OP is reporting this is failing somehow, right? And in some cases something meant to be a string is winding up unquoted, and being interpreted as something else on load? Or I'm probably still not quite getting it. Okay, yeah, tldr, we can stick with: YAML's data/processing model ends up being way too complex making it's behavior hard to predict and hard to maintain consistent between parsers/versions.