Skip to content

Commit b11dbcb

Browse files
eramongodbkevinAlbs
eramongodb
authored and
kevinAlbs
committed
Reduce expected removeKeyAltName operations to a single findOneAndUpdate (#1037)
* Update CSE unified spec tests to dade3e4d. * Implement removeKeyAltName as a single findOneAndUpdate operation * Fix null dereference of optional ignoreExtraEvents pointer
1 parent 8a7cd9b commit b11dbcb

File tree

3 files changed

+183
-123
lines changed

3 files changed

+183
-123
lines changed

‎src/libmongoc/src/mongoc/mongoc-client-side-encryption.c

Lines changed: 40 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -2358,11 +2358,34 @@ mongoc_client_encryption_remove_key_alt_name (
23582358

23592359
_mongoc_bson_init_if_set (key_doc);
23602360

2361+
23612362
{
23622363
mongoc_find_and_modify_opts_t *const opts =
23632364
mongoc_find_and_modify_opts_new ();
2364-
bson_t *const update =
2365-
BCON_NEW ("$pull", "{", "keyAltNames", BCON_UTF8 (keyaltname), "}");
2365+
2366+
/* clang-format off */
2367+
bson_t *const update = BCON_NEW (
2368+
"0", "{",
2369+
"$set", "{",
2370+
"keyAltNames", "{",
2371+
"$cond", "[",
2372+
"{",
2373+
"$eq", "[", "$keyAltNames", "[", keyaltname, "]", "]",
2374+
"}",
2375+
"$$REMOVE",
2376+
"{",
2377+
"$filter", "{",
2378+
"input", "$keyAltNames",
2379+
"cond", "{",
2380+
"$ne", "[", "$$this", keyaltname, "]",
2381+
"}",
2382+
"}",
2383+
"}",
2384+
"]",
2385+
"}",
2386+
"}",
2387+
"}");
2388+
/* clang-format on */
23662389

23672390
BSON_ASSERT (mongoc_find_and_modify_opts_set_update (opts, update));
23682391

@@ -2373,85 +2396,28 @@ mongoc_client_encryption_remove_key_alt_name (
23732396
mongoc_find_and_modify_opts_destroy (opts);
23742397
}
23752398

2376-
/* Ensure keyAltNames field is removed if it would otherwise be empty. */
2377-
if (ret) {
2399+
if (ret && key_doc) {
23782400
bson_iter_t iter;
2379-
bool should_remove = true;
23802401

2381-
BSON_ASSERT (bson_iter_init (&iter, &local_reply));
2402+
if (bson_iter_init_find (&iter, &local_reply, "value")) {
2403+
const bson_value_t *const value = bson_iter_value (&iter);
23822404

2383-
if (bson_iter_find_descendant (&iter, "value.keyAltNames", &iter)) {
2384-
if (!BSON_ITER_HOLDS_ARRAY (&iter)) {
2405+
if (value->value_type == BSON_TYPE_DOCUMENT) {
2406+
bson_t bson;
2407+
BSON_ASSERT (bson_init_static (
2408+
&bson, value->value.v_doc.data, value->value.v_doc.data_len));
2409+
bson_copy_to (&bson, key_doc);
2410+
bson_destroy (&bson);
2411+
} else if (value->value_type == BSON_TYPE_NULL) {
2412+
bson_t bson = BSON_INITIALIZER;
2413+
bson_copy_to (&bson, key_doc);
2414+
bson_destroy (&bson);
2415+
} else {
23852416
bson_set_error (error,
23862417
MONGOC_ERROR_CLIENT,
23872418
MONGOC_ERROR_CLIENT_INVALID_ENCRYPTION_STATE,
2388-
"expected keyAltNames to be an array of strings");
2419+
"expected field value to be a document or null");
23892420
ret = false;
2390-
GOTO (fail);
2391-
}
2392-
2393-
if (bson_iter_recurse (&iter, &iter)) {
2394-
while (bson_iter_next (&iter)) {
2395-
if (!BSON_ITER_HOLDS_UTF8 (&iter)) {
2396-
bson_set_error (
2397-
error,
2398-
MONGOC_ERROR_CLIENT,
2399-
MONGOC_ERROR_CLIENT_INVALID_ENCRYPTION_STATE,
2400-
"expected keyAltNames to be an array of strings");
2401-
ret = false;
2402-
GOTO (fail);
2403-
}
2404-
2405-
if (strcmp (bson_iter_utf8 (&iter, NULL), keyaltname) != 0) {
2406-
should_remove = false;
2407-
break;
2408-
}
2409-
}
2410-
}
2411-
} else {
2412-
/* If keyAltNames does not exist, do not try to remove it. */
2413-
should_remove = false;
2414-
}
2415-
2416-
if (should_remove) {
2417-
bson_t *update =
2418-
BCON_NEW ("$unset", "{", "keyAltNames", BCON_BOOL (true), "}");
2419-
bson_t reply;
2420-
2421-
ret = mongoc_collection_update_one (client_encryption->keyvault_coll,
2422-
&query,
2423-
update,
2424-
NULL,
2425-
&reply,
2426-
error);
2427-
2428-
bson_destroy (update);
2429-
bson_destroy (&reply);
2430-
}
2431-
2432-
if (ret && key_doc) {
2433-
bson_iter_t iter;
2434-
2435-
if (bson_iter_init_find (&iter, &local_reply, "value")) {
2436-
const bson_value_t *const value = bson_iter_value (&iter);
2437-
2438-
if (value->value_type == BSON_TYPE_DOCUMENT) {
2439-
bson_t bson;
2440-
BSON_ASSERT (bson_init_static (
2441-
&bson, value->value.v_doc.data, value->value.v_doc.data_len));
2442-
bson_copy_to (&bson, key_doc);
2443-
bson_destroy (&bson);
2444-
} else if (value->value_type == BSON_TYPE_NULL) {
2445-
bson_t bson = BSON_INITIALIZER;
2446-
bson_copy_to (&bson, key_doc);
2447-
bson_destroy (&bson);
2448-
} else {
2449-
bson_set_error (error,
2450-
MONGOC_ERROR_CLIENT,
2451-
MONGOC_ERROR_CLIENT_INVALID_ENCRYPTION_STATE,
2452-
"expected field value to be a document or null");
2453-
ret = false;
2454-
}
24552421
}
24562422
}
24572423
}

‎src/libmongoc/tests/json/client_side_encryption/unified/removeKeyAltName.json

Lines changed: 142 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,36 @@
118118
}
119119
}
120120
},
121-
"update": {
122-
"$pull": {
123-
"keyAltNames": "does_not_exist"
121+
"update": [
122+
{
123+
"$set": {
124+
"keyAltNames": {
125+
"$cond": [
126+
{
127+
"$eq": [
128+
"$keyAltNames",
129+
[
130+
"does_not_exist"
131+
]
132+
]
133+
},
134+
"$$REMOVE",
135+
{
136+
"$filter": {
137+
"input": "$keyAltNames",
138+
"cond": {
139+
"$ne": [
140+
"$$this",
141+
"does_not_exist"
142+
]
143+
}
144+
}
145+
}
146+
]
147+
}
148+
}
124149
}
125-
},
150+
],
126151
"writeConcern": {
127152
"w": "majority"
128153
}
@@ -239,11 +264,36 @@
239264
}
240265
}
241266
},
242-
"update": {
243-
"$pull": {
244-
"keyAltNames": "does_not_exist"
267+
"update": [
268+
{
269+
"$set": {
270+
"keyAltNames": {
271+
"$cond": [
272+
{
273+
"$eq": [
274+
"$keyAltNames",
275+
[
276+
"does_not_exist"
277+
]
278+
]
279+
},
280+
"$$REMOVE",
281+
{
282+
"$filter": {
283+
"input": "$keyAltNames",
284+
"cond": {
285+
"$ne": [
286+
"$$this",
287+
"does_not_exist"
288+
]
289+
}
290+
}
291+
}
292+
]
293+
}
294+
}
245295
}
246-
},
296+
],
247297
"writeConcern": {
248298
"w": "majority"
249299
}
@@ -378,11 +428,36 @@
378428
}
379429
}
380430
},
381-
"update": {
382-
"$pull": {
383-
"keyAltNames": "alternate_name"
431+
"update": [
432+
{
433+
"$set": {
434+
"keyAltNames": {
435+
"$cond": [
436+
{
437+
"$eq": [
438+
"$keyAltNames",
439+
[
440+
"alternate_name"
441+
]
442+
]
443+
},
444+
"$$REMOVE",
445+
{
446+
"$filter": {
447+
"input": "$keyAltNames",
448+
"cond": {
449+
"$ne": [
450+
"$$this",
451+
"alternate_name"
452+
]
453+
}
454+
}
455+
}
456+
]
457+
}
458+
}
384459
}
385-
},
460+
],
386461
"writeConcern": {
387462
"w": "majority"
388463
}
@@ -501,11 +576,36 @@
501576
}
502577
}
503578
},
504-
"update": {
505-
"$pull": {
506-
"keyAltNames": "alternate_name"
579+
"update": [
580+
{
581+
"$set": {
582+
"keyAltNames": {
583+
"$cond": [
584+
{
585+
"$eq": [
586+
"$keyAltNames",
587+
[
588+
"alternate_name"
589+
]
590+
]
591+
},
592+
"$$REMOVE",
593+
{
594+
"$filter": {
595+
"input": "$keyAltNames",
596+
"cond": {
597+
"$ne": [
598+
"$$this",
599+
"alternate_name"
600+
]
601+
}
602+
}
603+
}
604+
]
605+
}
606+
}
507607
}
508-
},
608+
],
509609
"writeConcern": {
510610
"w": "majority"
511611
}
@@ -525,42 +625,36 @@
525625
}
526626
}
527627
},
528-
"update": {
529-
"$pull": {
530-
"keyAltNames": "local_key"
531-
}
532-
},
533-
"writeConcern": {
534-
"w": "majority"
535-
}
536-
}
537-
}
538-
},
539-
{
540-
"commandStartedEvent": {
541-
"databaseName": "keyvault",
542-
"command": {
543-
"update": "datakeys",
544-
"updates": [
628+
"update": [
545629
{
546-
"q": {
547-
"_id": {
548-
"$binary": {
549-
"base64": "bG9jYWxrZXlsb2NhbGtleQ==",
550-
"subType": "04"
551-
}
552-
}
553-
},
554-
"u": {
555-
"$unset": {
556-
"keyAltNames": true
630+
"$set": {
631+
"keyAltNames": {
632+
"$cond": [
633+
{
634+
"$eq": [
635+
"$keyAltNames",
636+
[
637+
"local_key"
638+
]
639+
]
640+
},
641+
"$$REMOVE",
642+
{
643+
"$filter": {
644+
"input": "$keyAltNames",
645+
"cond": {
646+
"$ne": [
647+
"$$this",
648+
"local_key"
649+
]
650+
}
651+
}
652+
}
653+
]
557654
}
558655
}
559656
}
560-
],
561-
"writeConcern": {
562-
"w": "majority"
563-
}
657+
]
564658
}
565659
}
566660
}

‎src/libmongoc/tests/unified/runner.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1221,7 +1221,7 @@ test_check_expected_events_for_client (test_t *test,
12211221
LL_COUNT (entity->events, eiter, actual_num_events);
12221222
if (expected_num_events != actual_num_events) {
12231223
bool too_many_events = actual_num_events > expected_num_events;
1224-
if (*ignore_extra_events) {
1224+
if (ignore_extra_events && *ignore_extra_events) {
12251225
// We can never have too many events
12261226
too_many_events = false;
12271227
}

0 commit comments

Comments
 (0)