-
Notifications
You must be signed in to change notification settings - Fork 29
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
Default to safe=True? #21
Comments
Hi, thanks for comment, but are you sure |
Iirc it makes pyyaml output as simple types as possible, so that even dumbest stripped-down parsers can read it, i.e. "safe" in a "most compatible" sense. |
Which, iirc also makes resulting YAML as ugly as possible, which clearly contradicts the goal of the module. |
Also, totally agree on load vs safe_load, always use the latter myself, but don't think it's related, though will need to check it, not 100% sure. |
I don’t really think so. The issues with unsafe load are much more severe. |
But this module doesn't do loading at all! ;) |
Seen that :) I also noticed that you use |
Just noticed that pyaml, similarly, uses UnsafePrettyYAMLDumper, which dabbles with scalar and sequence styles, and can occasionally produce broken output, hence the option to avoid it. Don't think renaming it now is a good idea though, and it's at least somewhat consistent that way, if nothing else, so guess I'll just leave it be. |
Also, PrettyYAMLDumper and co always based on yaml.dumper.SafeDumper, and I dare you to look at the things with exclamation marks non-safe pyyaml dumper produces, it's just ridiculous. |
Sounds reasonable. Thanks for explaining. |
Currently,
pyaml.dump
defaults tosafe=False
. Maybe it could be changed tosafe=True
by default (security by default)?Would there be any noticeable difference from that and do you think
safe=True
would limit normal use cases?If you disagree with changing the default, maybe the docs can be updated to use the
safe=True
where possible?See also: Make load safe_load
Thanks for your nice library!
The text was updated successfully, but these errors were encountered: