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

Loading keyrings from unvalidated YAML files. #170

Open
Skogetroll opened this issue Jun 21, 2017 · 1 comment
Open

Loading keyrings from unvalidated YAML files. #170

Skogetroll opened this issue Jun 21, 2017 · 1 comment

Comments

@Skogetroll
Copy link
Contributor

There's problem with KeyringLiberator.get_keyring_at_path/KeyringLiberator. get_all_keyrings — it works pretty fine when all the files in the keys_dir folder are in exact format library needs, but in case any of them deviates from unwritten scheme all gets broken.

For example, our team had situation when one of YAML files in ~/.cocoapods/keys became empty(for unknown reasons), and when we tried to do pod install we had this:

NoMethodError - undefined method '[]' for false:FalseClass
~/.rvm/gems/ruby-2.4.1/gems/cocoapods-keys-2.0.0/lib/keyring.rb:14:in 'from_hash'
~/.rvm/gems/ruby-2.4.1/gems/cocoapods-keys-2.0.0/lib/keyring_liberator.rb:66:in 'get_keyring_at_path'
~/.rvm/gems/ruby-2.4.1/gems/cocoapods-keys-2.0.0/lib/keyring_liberator.rb:60:in 'block in get_all_keyrings'
~/.rvm/gems/ruby-2.4.1/gems/cocoapods-keys-2.0.0/lib/keyring_liberator.rb:59:in 'each'
~/.rvm/gems/ruby-2.4.1/gems/cocoapods-keys-2.0.0/lib/keyring_liberator.rb:59:in 'get_all_keyrings'
~/.rvm/gems/ruby-2.4.1/gems/cocoapods-keys-2.0.0/lib/keyring_liberator.rb:28:in 'get_current_keyring'
~/.rvm/gems/ruby-2.4.1/gems/cocoapods-keys-2.0.0/lib/preinstaller.rb:23:in 'setup'
… // rest of the callstack is not so important

And it crashes with this because YAML.load(in get_keyring_at_path) for empty file/string returns false, and in Keyring.from_hash there's attempt to perform subscript operator ([]) with false in hash parameter, because there's no checks on this parameter, neither type or keys are validated.

So, it is recommended to add some checks in Keyring.from_hash and similar methods to prevent such nuisance from happening.
Now any random invalid .yml file in the folder easily breaks plugin work.

@orta
Copy link
Owner

orta commented Jun 22, 2017

Weird - yeah, I'd definitely accept a PR that wraps the YML loader with a nicer error message for cases like this 👍

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