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 Gem::Specification#ruby_code handle OpenSSL::PKey::RSA objects #2782

Merged
4 commits merged into from Sep 17, 2019

Conversation

bronzdoc
Copy link
Member

Description:

closes #2776

@zenspider what you think?


I will abide by the code of conduct.

@bronzdoc bronzdoc changed the title Handle rsa keys in to ruby Make Gem::Specification#ruby_code handle OpenSSL::PKey::RSA objects May 30, 2019
@zenspider
Copy link
Contributor

That seems fine to me, but I suspect this is used to serialize specs still. Skipping outright might be a better option... I'd want someone more in the know to weigh in on that. I haven't maintained rubygems in ages.

But for MY purposes, as long as it doesn't crash I'm happy.

@zenspider
Copy link
Contributor

Also, thank you. That was very timely.

@deivid-rodriguez
Copy link
Member

I would vote for completely skipping this field too. As far as I know, this is only used at gem build time, and it's no longer needed once the deserialized spec file is installed to the final user system?

@bronzdoc
Copy link
Member Author

Removing the signing key trigger a couple of errors, and made me notice we are printing the cert_chain too but since it's an array it's handled by:

when Array              then '[' + obj.map { |x| ruby_code x }.join(", ") + ']'¬

so kinda the output we want to avoid by not doing to_s or to to_text from a OpenSSL::PKey::RSA object, should we skip the cert_chain attribute too?

@bronzdoc
Copy link
Member Author

Build failures are not related to this change, I don't understand why travis is having issues with bundler in this particular PR 🤔

@zenspider
Copy link
Contributor

The tests know how much I hate bundler and are getting back at me. 🙁

@MSP-Greg
Copy link
Contributor

I think this may be the result of the older version of bundler currently being used.

I believe the changes in rubygems/bundler@e742c3d / rubygems/bundler#7100 will fix this.

But, it breaks Ruby master/trunk/ruby-head builds when they are used for CI testing. That fix is at rubygems/bundler#7248

@zenspider
Copy link
Contributor

... status? There's been 4 releases since this came up...

@bronzdoc
Copy link
Member Author

@bundlerbot r+

ghost pushed a commit that referenced this pull request Sep 17, 2019
2782: Make Gem::Specification#ruby_code handle OpenSSL::PKey::RSA objects r=bronzdoc a=bronzdoc

# Description:
closes #2776

@zenspider what you think?
______________
I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).


Co-authored-by: bronzdoc <lsagastume1990@gmail.com>
@ghost
Copy link

ghost commented Sep 17, 2019

Build succeeded

@ghost ghost merged commit ba021fb into master Sep 17, 2019
@ghost ghost deleted the handle_rsa_keys_in_to_ruby branch September 17, 2019 05:11
@bronzdoc bronzdoc added this to the 3.1.0 milestone Sep 17, 2019
@hsbt hsbt modified the milestones: 3.1.0, 3.0.7 Oct 15, 2019
hsbt pushed a commit that referenced this pull request Feb 18, 2020
2782: Make Gem::Specification#ruby_code handle OpenSSL::PKey::RSA objects r=bronzdoc a=bronzdoc

# Description:
closes #2776

@zenspider what you think?
______________
I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).


Co-authored-by: bronzdoc <lsagastume1990@gmail.com>
This pull request was closed.
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

Successfully merging this pull request may close these issues.

#to_ruby isn't handling OpenSSL::PKey::RSA instances
5 participants