Conversation
sgolemon
requested review from
erwee,
kevinAlbs,
markbenvenuto and
shreyaskalyan
kevinAlbs
left a comment
kevinAlbs
left a comment
There was a problem hiding this comment.
Nice. LGTM. _test_ctx_wrap_and_feed_key looks helpful.
Consider adding tests to _test_encrypt_fle2_explicit for explicit encryption. I expect explicit encryption to "just work". Explicit encryption produces the placeholders. And the placeholders have not changed in FLE2v2.
src/mongocrypt-ctx-decrypt.c
Outdated
| CHECK_AND_RETURN (K_KeyId); | ||
| CHECK_AND_RETURN_KB_STATUS ( | ||
| _mongocrypt_key_broker_decrypted_key_by_id (kb, K_KeyId, &K_Key)); | ||
| CHECK_AND_RETURN (mc_FLE2IndexedEqualityEncryptedValue_add_K_Key ( |
There was a problem hiding this comment.
Not related to these changes. Suggest renaming mc_FLE2IndexedEqualityEncryptedValue_add_K_Key to mc_FLE2IndexedEncryptedValue_add_K_Key since it is used for both Equality and Range payloads.
There was a problem hiding this comment.
I did have the same thought, actually. Happy to include it.
sgolemon
left a comment
sgolemon
left a comment
There was a problem hiding this comment.
Addressed comments, then rebased now that MC-531 has landed.
src/mongocrypt-ctx-decrypt.c
Outdated
| CHECK_AND_RETURN (K_KeyId); | ||
| CHECK_AND_RETURN_KB_STATUS ( | ||
| _mongocrypt_key_broker_decrypted_key_by_id (kb, K_KeyId, &K_Key)); | ||
| CHECK_AND_RETURN (mc_FLE2IndexedEqualityEncryptedValue_add_K_Key ( |
There was a problem hiding this comment.
I did have the same thought, actually. Happy to include it.
|
Merged as 4a544db |
6 participants
This is notably missing an integration test for IUP-V2, but I'm waiting on that 'cause I think @shreyaskalyan is working on unit tests for it. Everything else seems to work smoothly though, so it's worth starting review since this is... growing.