arrow icon indicating copy to clipboard operation
arrow copied to clipboard

GH-40328: [C++][Parquet] Allow use of FileDecryptionProperties after the CryptoFactory is destroyed

Open adamreeve opened this issue 1 year ago • 0 comments

Rationale for this change

See #40328

What changes are included in this PR?

Introduces a new wrapper class that holds a FileKeyUnwrapper and a shared pointer to a KeyToolkit so that the FileKeyUnwrapper remains usable after the CryptoFactory that created it is destroyed, and changes CryptoFactory to store a shared pointer to a KeyToolkit instead of storing a KeyToolkit inline.

I'd be open to other suggestions on how to handle this, as this does feel like a slightly hacky workaround.

I considered making the FileKeyUnwrapper constructors take a std::shared_ptr<KeyToolkit> instead of KeyToolkit*, but this caused issues with KeyToolkit::RotateMasterKeys, which needs to construct a FileKeyUnwrapper using a reference to itself. I could possibly have worked around this by making KeyToolkit inherit from std::enable_shared_from_this, but KeyToolkit is publicly exported so it would be difficult to ensure it's always managed by a shared pointer. FileKeyUnwrapper is also publicly exported so this would be a breaking change, although it's not clear that these classes would be used by many users directly, I'd imagine most users would only interact with a CryptoFactory instance.

Are these changes tested?

Yes, I've added a new unit test for this

Are there any user-facing changes?

This changes functionality for users of the C++ API in a non-breaking way, allowing them to use the FileDecryptionProperties returned from a CryptoFactory without having to keep the CryptoFactory alive.

  • GitHub Issue: #40328

adamreeve avatar Mar 04 '24 00:03 adamreeve