Psych::DisallowedClass: Tried to load unspecified class: Time with Ruby 3.1 / Psych 4.0
#80
Comments
|
So: I observe the same failures, and I agree they're a problem. Your second approach seems more like a "real fix" to me, and the first approach more like a hack or workaround, like you say. Main problem here is that I'm no expert Ruby programmer, and know very little about Psych. I suspect we should focus on your second approach and see if that works out. I'm also pretty busy this weekend, and distracted by World News stuff. Give me a week or two to try to better understand this issue and what the ramifications of doing the date-to-string conversion are here? Or if you know the ramifications, feel free to expound here, and I'll follow your lead. Feel free to ping me here in a week if I haven't responded; I'm not great at managing my GitHub TODOs. :) |
|
Thx for your thoughts and no worries take your time. I think that the question here is backward compatibility, if somebody uses Ronn as a library. The workaround is keeping status quo and turn over the issue to the library users. The second approach is more owning the problem and admitting there is issue and providing fix with know consequences. The question is who use ronn-ng as a library? Just a quick look on ronn-ng, there is not many. Looking at original Ronn [2], the Mustache compatibility would be probably the question. [1] https://rubygems.org/gems/ronn-ng/reverse_dependencies |
|
So… what's the decision here? |
|
Well crap. I have no decision. I'm not good enough with Ruby to make a good call here without further study and experimentation. My view is: ronn-ng is an application that provides a command, and the fact that it is uses Ruby is an internal implementation detail. I wasn't intending to support use of ronn-ng as a library at all. Writing and maintaining a library is harder and more complex than maintaining an application/command. I don't know if this is the right view. I picked up Ronn as abandonware after the original author seemed to lose interest. I thought it was just for use as a command/program. But maybe that was wrong. @voxik 's suggestion of the My main point of reference is, when called as a command from an external process, does the To voxik's last question/post: I didn't know that there were any people that consumed ronn as a library. I wasn't intending to support that; I didn't know that was a thing that people did. But I also don't want to screw over existing users if they're doing that already. Sorry folks. I dunno really what to do here. |
|
Looking into this again, I think that the right solution is the
I'll submit PR with the |
|
That reasoning makes sense to me. Go for it. |
Since Psych 4.0, the `safe_load` is used as default loading mechanism. There are just a few permitted classes and `Time` is not one of them [[1]]. This results it test failure: ~~~ Error: test_converting_to_yaml(DocumentTest::TestSimpleConventionallyNamedDocument): Psych::DisallowedClass: Tried to load unspecified class: Time ~~~ Please also note that in YAML specs 1.2, the `timestamp` is not listed as supported tag anymore [[2]]. Given that: 1) ronn-ng does not provide any supported way of loading the serialized YAML. 2) The `to_yaml` does not appear to be used internally/externally anywhere. 2) If there were users of this functionality, it would have been already know, reported and fixed at this moment. The best course of action is fixing the test case by listing the `Time` as valid class for parsing. Fixes apjanke#80 [1]: https://docs.ruby-lang.org/en/master/Psych.html#method-c-safe_load [2]: yaml/yaml-spec#268
Since Psych 4.0, the `safe_load` is used as default loading mechanism. There are just a few permitted classes and `Time` is not one of them [[1]]. This results it test failure: ~~~ Error: test_converting_to_yaml(DocumentTest::TestSimpleConventionallyNamedDocument): Psych::DisallowedClass: Tried to load unspecified class: Time ~~~ Please also note that in YAML specs 1.2, the `timestamp` is not listed as supported tag anymore [[2]]. Given that: 1) ronn-ng does not provide any supported way of loading the serialized YAML. 2) The `to_yaml` does not appear to be used internally/externally anywhere. 3) If there were users of this functionality, it would have been already know, reported and fixed at this moment. The best course of action is fixing the test case by listing the `Time` as valid class for parsing. Fixes apjanke#80 [1]: https://docs.ruby-lang.org/en/master/Psych.html#method-c-safe_load [2]: yaml/yaml-spec#268
|
I think I've got this fixed with the changes I made for #87, and a Ronn-NG 0.10.1 release will be coming out shortly. Please re-open this bug report if you continue to have issues. |
Running Ronn test suite, I observe the following failure:
which seems to be related to ruby/psych#262.
There are several possible fixes. This is the easy one:
But that seems to be more of workaround the test error then proper fix.
The other possibility is:
Which would be more in line to what
to_jsondoes already. But that would also mean that the date is treated as a String in YAML.The text was updated successfully, but these errors were encountered: