How do I load this without monkey patching?
rkh opened this issue · 9 comments
This is currently unusable for libraries. :(
Honest question: how do you plan on using this in a library?
Originally, my intention was for safe_yaml to be useful to application developers. The monkey patching--which mastahyeti introduced way back in the first pull request, and I've stuck with--makes it very useful in this context: all you do is add a dependency on safe_yaml, and automatically your application is safer wherever your code or any code calls YAML.load.
My guess is that you have a library and you want it to deserialize YAML somewhere, and you want to use YAML.safe_load for that. Is that correct? I guess I can see three possible approaches here:
- Remove the monkey patching from safe_yaml and require an extra step for it (like
YAML.monkeypatch!or something less stupid). The reason I don't like this option is that it makes life just slightly harder for app devs, and I really like the fact that currently all this gem needs to work is a singlerequirestatement. - Add an option to reverse the monkey patch. This would be similar to setting
SafeYAML::OPTIONS[:default_mode] = :unsafe. This seems bad too, though, as you don't want to, as a library, potentially reverse the decision that an app developer already made. - Split the gem in two: yaml_safe_load (or something like that), which adds the functionality of safe_yaml without touching
YAMLitself; and safe_yaml, which depends on yaml_safe_load and does the monkey patching.
The third is the most work, but I kinda think it's the best approach. Of course there could be a fourth option I'm not thinking of.
I wrote a library for parsing TAP. TAP can contain YAML and is in most cases untrusted input. I would like to be able to require something in my library so that I can load YAML safely but don't mess with the YAML.load method for anything else. That is, if safe_yaml has been loaded by the app using my library, it should still be safe, otherwise it should still be able to load any unsafe objects. Therefore option 2 would not work. I would also prefer 3. It doesn't need to be a separate gem, it could also be that I load safe_yaml/safe_load instead of safe_yaml or something.
Ha, that's a good point. Pretty ridiculous that I didn't even think of that. It should be pretty easy to extract the safe_load functionality into a separate file; I will do that and try to get it pushed soon.
OK, I've pushed a change that I think should address your needs. Try updating your gem spec to temporarily pull from HEAD and use require safe_yaml/load. Let me know if that works for you and/or if you see any major issues with this approach.
I'm the same boat, using this for a library. Would love to push a release with this addition.
Would a confirmation of the behavior help a release of safe_yaml? Thanks @dtao!
@fnichol Sorry for the delayed response. Yeah, if you have tried out the safe_yaml/load solution and it works for you, that'd be good to know. The build is currently failing and I've been dragging my feet on investigating; but once I square that away and make sure this approach works for at least one person besides me (!) I intend on releasing the next version.
No problem, I'll attempt to give it a spin today or tomorrow to confirm. Thanks!
OK I'm closing this since require "safe_yaml/load" is available now in SafeYAML 1.0. If anybody experiences any problems with that, let me know and I will reopen!