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

Expose safe_loading #6

Open
lbolla opened this issue Jan 10, 2019 · 8 comments
Open

Expose safe_loading #6

lbolla opened this issue Jan 10, 2019 · 8 comments

Comments

@lbolla
Copy link
Owner

lbolla commented Jan 10, 2019

At the moment, yamlenv uses yaml.load to load .yaml files.
We should also support using yaml.safe_load.

@jeverling
Copy link

I'm not an expert at all in yaml parsing, but would it work as a workaround to use yml = yaml.safe_load(fp), then tmp = yaml.safe_dump(yml) and then yamlenv.load(tmp)?
I think this should prevent safety issues or am I wrong?

@lbolla
Copy link
Owner Author

lbolla commented Jun 3, 2019

@jeverling That's pretty inefficient, though. Also, I am not sure Python objects like tuples would be handled wrongly. E.g., see how load+dump a tuple transforms it in a list, when using "safe":

>>> yaml.safe_load(yaml.safe_dump((1,2,3)))
[1, 2, 3]
>>> yaml.load(yaml.dump((1,2,3)))
(1, 2, 3)

@jeverling
Copy link

Thanks, that makes sense!

@jeverling
Copy link

Thanks for the great project btw! I was looking at the way docker-compose interpolates variables in docker-compose.yml files, but your library is much nicer than trying to reuse what they do.

@lbolla
Copy link
Owner Author

lbolla commented Jun 3, 2019

Thanks! I am happy to accept a PR if you have time to work on it!

@jeverling
Copy link

Not right now, but I hope I can find the time in the next weeks.

@thisHermit
Copy link

Hey @lbolla, I was interested in working on this. I am not sure what to do though.
I need to solve to it for kibitzr over here.

Your help is much appreciated.

@lbolla
Copy link
Owner Author

lbolla commented Oct 16, 2019

@thisHermit I think supporting safe_load requires changing the Loader inheritance with SafeLoader.

Have a look at PyYaml docs on how to do it.

I'd be good to have a unittest that highlights the problem with the current implementation. I am OK with replacing the simple Loader with SafeLoader, btw: no need to support both, imo.

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

No branches or pull requests

3 participants