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

Safety first, unsafe second (s/load/unsafeLoad/g;s/safeLoad/load/g) #330

Closed
ypid opened this issue Feb 28, 2017 · 9 comments
Closed

Safety first, unsafe second (s/load/unsafeLoad/g;s/safeLoad/load/g) #330

ypid opened this issue Feb 28, 2017 · 9 comments

Comments

@ypid
Copy link

ypid commented Feb 28, 2017

Thanks very much for your work! As said in the README, the project started out as a port of PyYAML. The origins of js-yaml are still visible because some bad decisions in PyYAML have also made it into this rewrite. Mainly that the default load function is unsafe and the safe variant is called safeLoad.

Check the proposal in https://bitbucket.org/ruamel/yaml/issues/6/make-the-default-load-and-dump-methods is which I would also propose here. This would be a break, sure, but that is what semantic versioning is for 😉

Refer to yaml/pyyaml#5 which includes many examples and a talk why this is bad.

@ypid ypid changed the title Safety first, unsafe second (s/load/unsafe_load/g;s/safeLoad/load/g) Safety first, unsafe second (s/load/unsafeLoad/g;s/safeLoad/load/g) Feb 28, 2017
@ypid
Copy link
Author

ypid commented Feb 28, 2017

I just found #77. Sorry for the duplicate but could this be rehashed? @puzrin What stronger reason do you need than safety first and the once referred to above?

@puzrin
Copy link
Member

puzrin commented Feb 28, 2017

That issue in pyyaml is still open, because nobody cares :) I'd say, current situation has historic reasons. I would also prefer safe defaults, but that's not enougth to fuckup API. Also, probably JS implementation needs stringify/parse methods like JSON.

In short: no time for active maintenance of this project :). It's very boring to decide every design/'spec problem separately. Someone need to spend noticeable time and summarize all pending tails, related to design/spec, to move forward.

@ypid
Copy link
Author

ypid commented Feb 28, 2017

That issue in pyyaml is still open, because nobody cares :)

People do care, but most of the time only after something has happened. There are so many issues and CVEs about those kinds of bad API design. Checkout the actively maintained projects ruamel.yaml and strictyaml where those issues are addressed. PyYAML seems unmaintained, thats why they did not address it I guess. Please checkout https://www.youtube.com/watch?v=kjZHjvrAS74

In short: no time for active maintenance of this project :)

Can I submit a PR maybe and help with it?

@puzrin
Copy link
Member

puzrin commented Feb 28, 2017

Can I submit a PR maybe and help with it?

As i said, i would like to avoid major version change for only one feature. This package is stable and used wide. A lot of issues in tracker should be resolved to say "we did something new".

I mean:

  • what to do with maps/sets (JS-related restrictions and types mapping)
  • should we fall back to libyaml concept with events layer
  • and everything marked as "spec".

That does not require coding, but requires to talks with spec authors, and some thinking.

PS. I have ideas how to rewrite parser with use of typed arrays for ultra fast tokens store. But have no chances to start until architecture issues clarified.

@ypid
Copy link
Author

ypid commented Feb 28, 2017

As i said, i would like to avoid major version change for only one feature. This package is stable and used wide.

OK, understandable, thanks for explaining. As not much "coding" is required to solve this issue, maybe it can be tagged to be included in the next major version bump?

@puzrin
Copy link
Member

puzrin commented Feb 28, 2017

I can keep it open for guarantee, but all problems are known very well :). I can even create one collaborated ticket with all existing design problems, if someone decides to help with those.

@ypid
Copy link
Author

ypid commented Feb 28, 2017

I can keep it open for guarantee

Thanks very much!

if someone decides to help with those.

To much to maintain currently myself. Maybe I will come back to it.

@puzrin puzrin mentioned this issue Feb 28, 2017
9 tasks
@puzrin
Copy link
Member

puzrin commented Feb 28, 2017

See #331.

@puzrin
Copy link
Member

puzrin commented Dec 4, 2020

Solved by 1d7d7e9

@puzrin puzrin closed this as completed Dec 4, 2020
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

2 participants