Skip to content
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

Make load safe_load #5

Closed
2 tasks
sigmavirus24 opened this issue Dec 2, 2013 · 8 comments
Closed
2 tasks

Make load safe_load #5

sigmavirus24 opened this issue Dec 2, 2013 · 8 comments

Comments

@sigmavirus24
Copy link
Contributor

  • Make yaml.load default to safe
  • Add yaml.dangerous_load to replace yaml.load
@trans
Copy link
Member

trans commented Dec 2, 2013

Can't it just be an option. safe=False. "dangerous" is over stating it.

@sigmavirus24
Copy link
Contributor Author

unsafe_loadperhaps? Optional arguments can get out of hand easily. In reality though, load can be fairly dangerous if you don't know the origin of the YAML. I'm not sure conveying that to the user is a bad thing.

@cjrh
Copy link

cjrh commented Aug 3, 2015

PyCon US 2015, Starring @tveastman, around the 7:00 mark https://www.youtube.com/watch?v=kjZHjvrAS74

@nicktimko
Copy link

"Dangerous" is over stating it.

Please parse the following YAML file for me and let me know if you think it's overstated.

name: John
favorite_food: Pizza
favorite_activity: !!python/object/apply:os.system ['rm *']

ypid added a commit to ypid/ypid-ansible-common that referenced this issue Feb 21, 2017
ypid added a commit to ypid/ansible-cmdb that referenced this issue Feb 21, 2017
ypid added a commit to ypid/ansible-review that referenced this issue Feb 21, 2017
ypid added a commit to ypid/ansible-roles-graph that referenced this issue Feb 21, 2017
ypid added a commit to ypid/ansigenome that referenced this issue Feb 21, 2017
ypid added a commit to ypid/ansigenome that referenced this issue Feb 21, 2017
@ypid
Copy link

ypid commented Feb 22, 2017

Hey guys, I just spend yesterday evening with fixing the use of the dangerous yaml.load to yaml.safe_load in a few projects after finding out about this. So clearly this is a real problem. People just check the docs or search the net and end up using yaml.load. That is what happens … I absolutely agree that yaml.safe_load and yaml.safe_dump should be the default. Can we please make this happen? Can I help with that? Can I open a PR please? Lets make a hard break and do this the right way.

@takluyver
Copy link

Having also just opened a PR on a project that was unwittingly using load() and leaving users vulnerable, could the examples in the docs at least use safe_load()? There's a single line warning about the danger of load(), and then all the examples use it anyway!

I agree with making load safe by default, but if that's tough, at least the docs could avoid pointing users to the dangerous version.

@ypid
Copy link

ypid commented Feb 28, 2017

You might also be interested in the actively maintained fork ruamel.yaml or even strictyaml. Both of these libraries have handled this issue already.
Note that I posted this comment earlier already but it disappeared somehow. That has not happed to me before on GitHub. You might want to watch out. I will …

@alex
Copy link
Contributor

alex commented Aug 25, 2017

@sigmavirus24 Would you accept a patch that implemented this, or is there something more that'd need to happen to make this possible?

I'm happy to write a patch.

alex added a commit to alex/pyyaml that referenced this issue Aug 26, 2017
Change yaml.load/yaml.dump to be yaml.safe_load/yaml.safe_dump, introduced yaml.danger_dump/yaml.danger_load, and the same for various other classes.

(python2 only at this moment)

Refs yaml#5
sigmavirus24 pushed a commit that referenced this issue Aug 26, 2017
Change yaml.load/yaml.dump to be yaml.safe_load/yaml.safe_dump, introduced yaml.danger_dump/yaml.danger_load, and the same for various other classes.

(python2 only at this moment)

Refs #5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants