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

Bump supported Ruby versions #57

Merged
merged 1 commit into from
Dec 2, 2019
Merged

Conversation

mbyczkowski
Copy link
Contributor

  • Bumped MRI Rubies to the ones that are still supported
  • Bumped JRuby to the last 2 releases. This required a change to how
    Java certificates are created in test (sun.security.x509.X509CertImpl
    can no longer be directly instantiated).

@mbyczkowski mbyczkowski force-pushed the mbyczkowski/bump-rubies branch 2 times, most recently from 7023337 to 8dba176 Compare November 25, 2019 22:34
Comment on lines -11 to +9
return unless certs
OpenSSL::X509::Certificate.new(extract_der(certs[0])).freeze
end

private

def extract_der(cert)
stringio = StringIO.new
cert.derEncode(stringio.to_outputstream)
stringio.string
return if certs.nil? || certs.empty?
OpenSSL::X509::Certificate.new(certs[0].get_encoded).freeze
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change backwards compatible?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it uses a more generic java.security.cert.Certificate abstract class to get the encoding (which has been around in Java for a looong time), rather than expect the instance of sun.security.x509.X509CertImpl.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, thanks!

spec/rails/auth/x509/middleware_spec.rb Outdated Show resolved Hide resolved
@mbyczkowski mbyczkowski force-pushed the mbyczkowski/bump-rubies branch 4 times, most recently from b1f923f to 3b12228 Compare November 26, 2019 06:07
env: JRUBY_OPTS="--debug" # for simplecov
- rvm: jruby-9.1
env: JRUBY_OPTS="--debug" # for simplecov
- rvm: jruby
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drcapulet After reading JRuby Roadmap, I think we should only support jruby-9.2 here (which is equivalent to jruby for now). Thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable!

@mbyczkowski mbyczkowski mentioned this pull request Nov 26, 2019
@mbyczkowski mbyczkowski force-pushed the mbyczkowski/bump-rubies branch 2 times, most recently from df66dc8 to de7b23a Compare November 26, 2019 20:31
@@ -29,6 +29,6 @@ Gem::Specification.new do |spec|

spec.add_runtime_dependency "rack"

spec.add_development_dependency "bundler", "~> 1.10"
spec.add_development_dependency "bundler", [">= 1.10", "< 3"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: make this just spec.add_development_dependency "bundler", ">= 1.10", "< 3"

Comment on lines -11 to +9
return unless certs
OpenSSL::X509::Certificate.new(extract_der(certs[0])).freeze
end

private

def extract_der(cert)
stringio = StringIO.new
cert.derEncode(stringio.to_outputstream)
stringio.string
return if certs.nil? || certs.empty?
OpenSSL::X509::Certificate.new(certs[0].get_encoded).freeze
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, thanks!

env: JRUBY_OPTS="--debug" # for simplecov
- rvm: jruby-9.1
env: JRUBY_OPTS="--debug" # for simplecov
- rvm: jruby
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable!

- Bumped MRI Rubies to the ones that are still supported
- Bumped JRuby to the last release (as it's recommended by JRuby's
  Roadmap). This also required a change to how Java certificates
  are created in test (sun.security.x509.X509CertImpl can no longer
  be directly instantiated).
- Allowed the use of Bundler 2
@mbyczkowski mbyczkowski merged commit e30b519 into master Dec 2, 2019
@mbyczkowski mbyczkowski deleted the mbyczkowski/bump-rubies branch December 2, 2019 18:08
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.

None yet

2 participants