Add Marshal support to X509 objects

[bdewater]

This allows for example to use Rails' cache to store these objects. Without this patch you'd get errors like TypeError (no _dump_data is defined for class OpenSSL::X509::Certificate).

Note that the X509::Revoked class doesn't need the newly introduced modules as the DER output of X509::CRL already includes these.

https://ruby-doc.org/core-2.6.5/Marshal.html

[ioquatix]

Rather than tacking the specs onto existing ones, do you think it would be a good idea to make them separate? I don't really mind, just back at you - what do you think?

[bdewater]

You're right, that is cleaner (and was a bit lazy on my part). Changed it!

[ioquatix]

I only have one other question. Is it canonical to use two separate modules to do this? Are there any other examples of Ruby code which do the same thing?

[grzuy]

Most tests are only testing that dump and load are reverse operations.

Would it help to have a test actually testing that it dumps to a string?
If not, you would have green tests by changing the implementation to anything that keeps operations reverse, like both being the "identity" operation e.g., right?

[bdewater]

Would it help to have a test actually testing that it dumps to a string?

Maybe I'm misunderstanding but I don't think we really care about the intermediate format, but more about the fact that the deserialized object is equal to the one we had before serializing it. The current way relies on the equality tests covering that, but I can add some of that to the marshal tests as an extra sanity check.

[grzuy]

Fair enough 👍

[bdewater]

I only have one other question. Is it canonical to use two separate modules to do this? Are there any other examples of Ruby code which do the same thing?

_load needs to be a class method, but I can change it to this as well so it's a single include line:

module Marshal
  def self.included(base)
    base.extend(ClassMethods)
  end

  module ClassMethods
    def _load(string)
      new(string)
    end
  end

  def _dump(_level)
    to_der
  end
end

[bdewater]

Addressed both bits of feedback.

[mperham]

Is there a way to tell which Ruby versions have a given OpenSSL PR like this one?

[bdewater]

@mperham this is in 2.8.0 trunk AFAIK, gem version 2.2.0 hasn't been released yet. I think a defined?(OpenSSL::X509::Marshal) ought to do the trick so you're not dependant on a version number?

[mperham]

Thanks, do things like PKeys also marshal?

…

On Apr 18, 2020, at 14:54, Bart de Water ***@***.***> wrote:  @mperham this is in 2.8.0 trunk AFAIK, gem version 2.2.0 hasn't been released yet. I think a defined?(OpenSSL::X509::Marshal) ought to do the trick so you're not dependant on a version number? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[bdewater]

Not right now.