Skip to content
This repository was archived by the owner on Apr 14, 2021. It is now read-only.

Output OpenSSL information only when OpenSSL is available.#6686

Merged
1 commit merged intorubygems:masterfrom
voxik:env-defined-openssl
Sep 25, 2018
Merged

Output OpenSSL information only when OpenSSL is available.#6686
1 commit merged intorubygems:masterfrom
voxik:env-defined-openssl

Conversation

@voxik
Copy link
Copy Markdown
Contributor

@voxik voxik commented Sep 6, 2018

It seems that only single OpenSSL availability check should be enough to output or not output the OpenSSL information.

I split this into two commits, so you can cherry-pick, in case you want to be more cautious about the availability of specific constants, but since they were introduced in Ruby 1.8.0 (ruby/ruby@78ff3833fb67c8005a9b851037e7), I would not bother.

Please note that that this code was pointed out by Coverity scanner (although it is definitely not an error):

Error: FORWARD_NULL (CWE-476):
rubygem-bundler-1.16.1/usr/share/gems/gems/bundler-1.16.1/lib/bundler/env.rb:113: null_check: Calling "defined?(OpenSSL)" implies that "OpenSSL" might be null-like.
rubygem-bundler-1.16.1/usr/share/gems/gems/bundler-1.16.1/lib/bundler/env.rb:114: property_access: Accessing a property of null-like value "OpenSSL".
#  112|         out << ["  Bin Dir", Gem.bindir]
#  113|         out << ["OpenSSL"] if defined?(OpenSSL)
#  114|->       out << ["  Compiled", OpenSSL::OPENSSL_VERSION] if defined?(OpenSSL::OPENSSL_VERSION)
#  115|         out << ["  Loaded", OpenSSL::OPENSSL_LIBRARY_VERSION] if defined?(OpenSSL::OPENSSL_LIBRARY_VERSION)
#  116|         out << ["  Cert File", OpenSSL::X509::DEFAULT_CERT_FILE] if defined?(OpenSSL::X509::DEFAULT_CERT_FILE)

@voxik
Copy link
Copy Markdown
Contributor Author

voxik commented Sep 6, 2018

Hm, the 1.8.7 behavior is interesting indeed. There should be probably loaded some additional library because although OpenSSL is defined, the OpenSSL constants are not available. So either just pick the first commit or fix Bundler to load what is necessary to load, because I don't know why the versions and paths should not be reported correctly on 1.8.7.

@colby-swandale
Copy link
Copy Markdown
Member

colby-swandale commented Sep 8, 2018

It's hard to accept this PR. We need to check the existence of each constant because some OpenSSL constants may not be defined in earlier versions of Ruby. The value of checking for the OpenSSL constant does not have much value.

@voxik
Copy link
Copy Markdown
Contributor Author

voxik commented Sep 9, 2018

The value of checking for the OpenSSL constant does not have much value.

The check was always there, it is just different now. Would it be acceptable if I dropped the second commit? Anyway feel free to reject this PR, I just thought I'll give it try.

@segiddins
Copy link
Copy Markdown
Contributor

segiddins commented Sep 10, 2018

Just the first commit would be perfect, thanks!

@colby-swandale
Copy link
Copy Markdown
Member

colby-swandale commented Sep 17, 2018

ping @voxik

@voxik voxik force-pushed the env-defined-openssl branch from b2a821c to 2bb4455 Compare September 18, 2018 06:53
@voxik
Copy link
Copy Markdown
Contributor Author

voxik commented Sep 18, 2018

I have dropped the second commit.

@segiddins
Copy link
Copy Markdown
Contributor

segiddins commented Sep 25, 2018

@bundlerbot r+

ghost pushed a commit that referenced this pull request Sep 25, 2018
@bundlerbot @voxik
Merge #6686
0d7259e 
6686: Output OpenSSL information only when OpenSSL is available. r=segiddins a=voxik

It seems that only single OpenSSL availability check should be enough to output or not output the OpenSSL information.

I split this into two commits, so you can cherry-pick, in case you want to be more cautious about the availability of specific constants, but since they were introduced in Ruby 1.8.0 (ruby/ruby@78ff3833fb67c8005a9b851037e7), I would not bother.

Please note that that this code was pointed out by Coverity scanner (although it is definitely not an error):

~~~
Error: FORWARD_NULL (CWE-476):
rubygem-bundler-1.16.1/usr/share/gems/gems/bundler-1.16.1/lib/bundler/env.rb:113: null_check: Calling "defined?(OpenSSL)" implies that "OpenSSL" might be null-like.
rubygem-bundler-1.16.1/usr/share/gems/gems/bundler-1.16.1/lib/bundler/env.rb:114: property_access: Accessing a property of null-like value "OpenSSL".
#  112|         out << ["  Bin Dir", Gem.bindir]
#  113|         out << ["OpenSSL"] if defined?(OpenSSL)
#  114|->       out << ["  Compiled", OpenSSL::OPENSSL_VERSION] if defined?(OpenSSL::OPENSSL_VERSION)
#  115|         out << ["  Loaded", OpenSSL::OPENSSL_LIBRARY_VERSION] if defined?(OpenSSL::OPENSSL_LIBRARY_VERSION)
#  116|         out << ["  Cert File", OpenSSL::X509::DEFAULT_CERT_FILE] if defined?(OpenSSL::X509::DEFAULT_CERT_FILE)
~~~

Co-authored-by: Vít Ondruch <vondruch@redhat.com>
@ghost
Copy link
Copy Markdown

ghost commented Sep 25, 2018

Build succeeded

@ghost ghost merged commit 2bb4455 into rubygems:master Sep 25, 2018
@colby-swandale colby-swandale added this to the 1.16.6 milestone Oct 2, 2018
colby-swandale pushed a commit that referenced this pull request Oct 5, 2018
@bundlerbot @voxik
Merge #6686
2396f95 
6686: Output OpenSSL information only when OpenSSL is available. r=segiddins a=voxik

It seems that only single OpenSSL availability check should be enough to output or not output the OpenSSL information.

I split this into two commits, so you can cherry-pick, in case you want to be more cautious about the availability of specific constants, but since they were introduced in Ruby 1.8.0 (ruby/ruby@78ff3833fb67c8005a9b851037e7), I would not bother.

Please note that that this code was pointed out by Coverity scanner (although it is definitely not an error):

~~~
Error: FORWARD_NULL (CWE-476):
rubygem-bundler-1.16.1/usr/share/gems/gems/bundler-1.16.1/lib/bundler/env.rb:113: null_check: Calling "defined?(OpenSSL)" implies that "OpenSSL" might be null-like.
rubygem-bundler-1.16.1/usr/share/gems/gems/bundler-1.16.1/lib/bundler/env.rb:114: property_access: Accessing a property of null-like value "OpenSSL".
#  112|         out << ["  Bin Dir", Gem.bindir]
#  113|         out << ["OpenSSL"] if defined?(OpenSSL)
#  114|->       out << ["  Compiled", OpenSSL::OPENSSL_VERSION] if defined?(OpenSSL::OPENSSL_VERSION)
#  115|         out << ["  Loaded", OpenSSL::OPENSSL_LIBRARY_VERSION] if defined?(OpenSSL::OPENSSL_LIBRARY_VERSION)
#  116|         out << ["  Cert File", OpenSSL::X509::DEFAULT_CERT_FILE] if defined?(OpenSSL::X509::DEFAULT_CERT_FILE)
~~~

Co-authored-by: Vít Ondruch <vondruch@redhat.com>
(cherry picked from commit 0d7259e)
colby-swandale pushed a commit that referenced this pull request Oct 5, 2018
@bundlerbot @voxik
Merge #6686
fc4e0e3 
6686: Output OpenSSL information only when OpenSSL is available. r=segiddins a=voxik

It seems that only single OpenSSL availability check should be enough to output or not output the OpenSSL information.

I split this into two commits, so you can cherry-pick, in case you want to be more cautious about the availability of specific constants, but since they were introduced in Ruby 1.8.0 (ruby/ruby@78ff3833fb67c8005a9b851037e7), I would not bother.

Please note that that this code was pointed out by Coverity scanner (although it is definitely not an error):

~~~
Error: FORWARD_NULL (CWE-476):
rubygem-bundler-1.16.1/usr/share/gems/gems/bundler-1.16.1/lib/bundler/env.rb:113: null_check: Calling "defined?(OpenSSL)" implies that "OpenSSL" might be null-like.
rubygem-bundler-1.16.1/usr/share/gems/gems/bundler-1.16.1/lib/bundler/env.rb:114: property_access: Accessing a property of null-like value "OpenSSL".
#  112|         out << ["  Bin Dir", Gem.bindir]
#  113|         out << ["OpenSSL"] if defined?(OpenSSL)
#  114|->       out << ["  Compiled", OpenSSL::OPENSSL_VERSION] if defined?(OpenSSL::OPENSSL_VERSION)
#  115|         out << ["  Loaded", OpenSSL::OPENSSL_LIBRARY_VERSION] if defined?(OpenSSL::OPENSSL_LIBRARY_VERSION)
#  116|         out << ["  Cert File", OpenSSL::X509::DEFAULT_CERT_FILE] if defined?(OpenSSL::X509::DEFAULT_CERT_FILE)
~~~

Co-authored-by: Vít Ondruch <vondruch@redhat.com>
(cherry picked from commit 0d7259e)
colby-swandale pushed a commit that referenced this pull request Oct 7, 2018
Merge branch '1-16-stable'
477115c 
* 1-16-stable:
  Version 1.16.6 with changelog
  fix uninitialized @use_gvp instance var warning
  no longer test Ruby 1.9.3 against rubygems master
  Merge #6708
  Auto merge of #6697 - walf443:added_changelog_section, r=hsbt
  Merge #6687
  Merge #6686
  Auto merge of #6670 - bundler:colby/invite-stephanie-morillo, r=segiddins
  Auto merge of #6627 - agrim123:agr-fix-add-groups, r=deivid-rodriguez
  Auto merge of #6612 - hdf1986:readme-bundle-add, r=segiddins
  Auto merge of #6495 - bundler:segiddins/6491-extra-gem-platform-in-lockfile, r=segiddins
  Auto merge of #6493 - agrim123:agr-update-bundle-update-docs, r=colby-swandale
  Auto merge of #6310 - utilum:rescue_unspecified_exception, r=segiddins
  Auto merge of #6184 - arbonap:pa-check-in-gemfile-docs, r=indirect
  fix typo
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.16.6

Development

Successfully merging this pull request may close these issues.

3 participants

@voxik @colby-swandale @segiddins