From 2ae0ebdf039674d1e92a40619449d93416fcf24e Mon Sep 17 00:00:00 2001 From: Kevin Albertson Date: Wed, 31 May 2023 14:16:48 -0400 Subject: [PATCH 1/8] prohibit extra fields in command subdocuments --- src/libmongoc/tests/json-test-monitoring.c | 78 ++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/src/libmongoc/tests/json-test-monitoring.c b/src/libmongoc/tests/json-test-monitoring.c index 90a6d04468b..33562ef37a3 100644 --- a/src/libmongoc/tests/json-test-monitoring.c +++ b/src/libmongoc/tests/json-test-monitoring.c @@ -336,6 +336,36 @@ apm_match_visitor (match_ctx_t *ctx, visitor_ctx->command_name = bson_strdup (bson_iter_key (doc_iter)); } + // Subdocuments in `command` are expected not to not have extra fields. + if (NULL != strstr (ctx->path, ".command") && doc_iter) { + if (BSON_ITER_HOLDS_DOCUMENT (doc_iter) && + BSON_ITER_HOLDS_DOCUMENT (pattern_iter)) { + bson_t doc_subdoc; + bson_iter_bson (doc_iter, &doc_subdoc); + bson_iter_t doc_subdoc_iter; + bson_iter_init (&doc_subdoc_iter, &doc_subdoc); + while (bson_iter_next (&doc_subdoc_iter)) { + const char *subdoc_key = bson_iter_key (&doc_subdoc_iter); + + bson_t pattern_subdoc; + bson_iter_bson (pattern_iter, &pattern_subdoc); + bson_iter_t pattern_subdoc_iter; + + if (!bson_iter_init_find ( + &pattern_subdoc_iter, &pattern_subdoc, subdoc_key)) { + match_err ( + ctx, + "unexpected extra field '%s' in captured event " + "command subdocument of field '%s'. pattern_subdoc=%s", + subdoc_key, + key, + tmp_json (&pattern_subdoc)); + return MATCH_ACTION_ABORT; + } + } + } + } + if (IS_COMMAND ("find") || IS_COMMAND ("aggregate")) { /* New query. Next server reply or getMore will set cursor_id. */ visitor_ctx->cursor_id = 0; @@ -617,9 +647,57 @@ test_apm_matching (void) apm_match_visitor_ctx_reset (&match_visitor_ctx); } +// Test that documents in command_started_event.command do not permit extra +// fields by default. +static void +test_apm_matching_extra_fields (void) +{ + // An extra value in `command` is permitted. + { + apm_match_visitor_ctx_t match_visitor_ctx = {0}; + match_ctx_t match_ctx = {{0}}; + + const char *event = BSON_STR ( + {"command_started_event" : {"command" : {"a" : 1, "b" : 2}}}); + const char *pattern = + BSON_STR ({"command_started_event" : {"command" : {"a" : 1}}}); + + match_ctx.visitor_fn = apm_match_visitor; + match_ctx.visitor_ctx = (void *) &match_visitor_ctx; + + bool matched = + match_bson_with_ctx (tmp_bson (event), tmp_bson (pattern), &match_ctx); + ASSERT (matched); + apm_match_visitor_ctx_reset (&match_visitor_ctx); + } + + // A subdocument in `command` does not permit extra values. + { + apm_match_visitor_ctx_t match_visitor_ctx = {0}; + match_ctx_t match_ctx = {{0}}; + + const char *event = BSON_STR ({ + "command_started_event" : {"command" : {"subdoc" : {"a" : 1, "b" : 2}}} + }); + const char *pattern = BSON_STR ( + {"command_started_event" : {"command" : {"subdoc" : {"a" : 1}}}}); + + match_ctx.visitor_fn = apm_match_visitor; + match_ctx.visitor_ctx = (void *) &match_visitor_ctx; + + bool matched = + match_bson_with_ctx (tmp_bson (event), tmp_bson (pattern), &match_ctx); + ASSERT (!matched); + ASSERT_CONTAINS (match_ctx.errmsg, "unexpected extra field 'b'"); + apm_match_visitor_ctx_reset (&match_visitor_ctx); + } +} + void test_apm_install (TestSuite *suite) { TestSuite_Add (suite, "/apm_test_matching", test_apm_matching); + TestSuite_Add ( + suite, "/apm_test_matching/extra_fields", test_apm_matching_extra_fields); } From 8666a7e924de149be5e23ea25fa4a5637b32c937 Mon Sep 17 00:00:00 2001 From: Kevin Albertson Date: Wed, 31 May 2023 14:17:57 -0400 Subject: [PATCH 2/8] call visitor_fn when matching arrays To check documents in arrays for extra fields. --- src/libmongoc/tests/json-test-monitoring.c | 23 ++++++++++++++++++++++ src/libmongoc/tests/test-conveniences.c | 10 ++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/libmongoc/tests/json-test-monitoring.c b/src/libmongoc/tests/json-test-monitoring.c index 33562ef37a3..211787fa037 100644 --- a/src/libmongoc/tests/json-test-monitoring.c +++ b/src/libmongoc/tests/json-test-monitoring.c @@ -691,6 +691,29 @@ test_apm_matching_extra_fields (void) ASSERT_CONTAINS (match_ctx.errmsg, "unexpected extra field 'b'"); apm_match_visitor_ctx_reset (&match_visitor_ctx); } + + // A subdocument in a subarray in `command` does not permit extra values. + { + apm_match_visitor_ctx_t match_visitor_ctx = {0}; + match_ctx_t match_ctx = {{0}}; + + const char *event = BSON_STR ({ + "command_started_event" : + {"command" : {"subarray" : [ {"a" : 1, "b" : 2} ]}} + }); + const char *pattern = BSON_STR ({ + "command_started_event" : {"command" : {"subarray" : [ {"a" : 1} ]}} + }); + + match_ctx.visitor_fn = apm_match_visitor; + match_ctx.visitor_ctx = (void *) &match_visitor_ctx; + + bool matched = + match_bson_with_ctx (tmp_bson (event), tmp_bson (pattern), &match_ctx); + ASSERT (!matched); + ASSERT_CONTAINS (match_ctx.errmsg, "unexpected extra field 'b'"); + apm_match_visitor_ctx_reset (&match_visitor_ctx); + } } diff --git a/src/libmongoc/tests/test-conveniences.c b/src/libmongoc/tests/test-conveniences.c index e0d20609cbc..dbc3e61de78 100644 --- a/src/libmongoc/tests/test-conveniences.c +++ b/src/libmongoc/tests/test-conveniences.c @@ -1302,6 +1302,16 @@ match_bson_arrays (const bson_t *array, const bson_t *pattern, match_ctx_t *ctx) derive (ctx, &derived, bson_iter_key (&array_iter)); + if (ctx->visitor_fn) { + match_action_t action = + ctx->visitor_fn (ctx, &pattern_iter, &array_iter); + if (action == MATCH_ACTION_ABORT) { + return false; + } else if (action == MATCH_ACTION_SKIP) { + continue; + } + } + if (!match_bson_value (array_value, pattern_value, &derived)) { return false; } From 2674d508827762d1b7d17b26570723b84dc44178 Mon Sep 17 00:00:00 2001 From: Kevin Albertson Date: Wed, 31 May 2023 14:35:20 -0400 Subject: [PATCH 3/8] copy fle2v2-CreateCollection from specifications commit 11af5ba6f2fea610940ddc644d3cdef54a697e73 --- .../legacy/fle2v2-CreateCollection.json | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/libmongoc/tests/json/client_side_encryption/legacy/fle2v2-CreateCollection.json b/src/libmongoc/tests/json/client_side_encryption/legacy/fle2v2-CreateCollection.json index 819d2eec3c4..cc8bd171451 100644 --- a/src/libmongoc/tests/json/client_side_encryption/legacy/fle2v2-CreateCollection.json +++ b/src/libmongoc/tests/json/client_side_encryption/legacy/fle2v2-CreateCollection.json @@ -158,9 +158,6 @@ "command": { "create": "encryptedCollection", "encryptedFields": { - "escCollection": null, - "ecocCollection": null, - "eccCollection": null, "fields": [ { "path": "firstName", @@ -343,9 +340,6 @@ "command": { "create": "encryptedCollection", "encryptedFields": { - "escCollection": null, - "ecocCollection": null, - "eccCollection": null, "fields": [ { "path": "firstName", @@ -851,9 +845,6 @@ "command": { "create": "encryptedCollection", "encryptedFields": { - "escCollection": null, - "ecocCollection": null, - "eccCollection": null, "fields": [ { "path": "firstName", @@ -1048,9 +1039,6 @@ "command": { "create": "encryptedCollection", "encryptedFields": { - "escCollection": null, - "ecocCollection": null, - "eccCollection": null, "fields": [ { "path": "firstName", @@ -1367,9 +1355,6 @@ "command": { "create": "encryptedCollection", "encryptedFields": { - "escCollection": null, - "ecocCollection": null, - "eccCollection": null, "fields": [ { "path": "firstName", @@ -1635,9 +1620,6 @@ "command": { "create": "encryptedCollection", "encryptedFields": { - "escCollection": null, - "ecocCollection": null, - "eccCollection": null, "fields": [ { "path": "firstName", From f68abda44d8d75ac146e20ca6102cc5f6b7cfad1 Mon Sep 17 00:00:00 2001 From: Kevin Albertson Date: Thu, 1 Jun 2023 09:44:07 -0400 Subject: [PATCH 4/8] skip check for on `multi` and `upsert` in updates --- src/libmongoc/tests/json-test-monitoring.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/libmongoc/tests/json-test-monitoring.c b/src/libmongoc/tests/json-test-monitoring.c index 211787fa037..055a8f6c2d2 100644 --- a/src/libmongoc/tests/json-test-monitoring.c +++ b/src/libmongoc/tests/json-test-monitoring.c @@ -351,8 +351,19 @@ apm_match_visitor (match_ctx_t *ctx, bson_iter_bson (pattern_iter, &pattern_subdoc); bson_iter_t pattern_subdoc_iter; - if (!bson_iter_init_find ( - &pattern_subdoc_iter, &pattern_subdoc, subdoc_key)) { + bool skip = false; + if (ends_with (ctx->path, "updates") && + (0 == strcmp ("multi", subdoc_key) || + 0 == strcmp ("upsert", subdoc_key))) { + // libmongoc includes `multi: false` and `upsert: false`. + // Some tests do not include `multi: false` and `upsert: false` + // in expectations. See DRIVERS-2271 and DRIVERS-976. + skip = true; + } + + if (!skip && !bson_iter_init_find (&pattern_subdoc_iter, + &pattern_subdoc, + subdoc_key)) { match_err ( ctx, "unexpected extra field '%s' in captured event " From 961982ae8bc0fabd7132a90134037752384c7e84 Mon Sep 17 00:00:00 2001 From: Kevin Albertson Date: Wed, 7 Jun 2023 15:26:43 -0400 Subject: [PATCH 5/8] silence scan-build --- src/libmongoc/tests/test-conveniences.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libmongoc/tests/test-conveniences.c b/src/libmongoc/tests/test-conveniences.c index dbc3e61de78..a74154f7e97 100644 --- a/src/libmongoc/tests/test-conveniences.c +++ b/src/libmongoc/tests/test-conveniences.c @@ -1302,7 +1302,7 @@ match_bson_arrays (const bson_t *array, const bson_t *pattern, match_ctx_t *ctx) derive (ctx, &derived, bson_iter_key (&array_iter)); - if (ctx->visitor_fn) { + if (ctx && ctx->visitor_fn) { match_action_t action = ctx->visitor_fn (ctx, &pattern_iter, &array_iter); if (action == MATCH_ACTION_ABORT) { From c737c72f8c364146b719b92a7ee2bd933cc37de1 Mon Sep 17 00:00:00 2001 From: Kevin Albertson Date: Thu, 8 Jun 2023 15:46:07 -0400 Subject: [PATCH 6/8] Apply suggested comment rewordings Co-authored-by: Ezra Chung <88335979+eramongodb@users.noreply.github.com> --- src/libmongoc/tests/json-test-monitoring.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libmongoc/tests/json-test-monitoring.c b/src/libmongoc/tests/json-test-monitoring.c index 055a8f6c2d2..b27b6de2411 100644 --- a/src/libmongoc/tests/json-test-monitoring.c +++ b/src/libmongoc/tests/json-test-monitoring.c @@ -336,7 +336,7 @@ apm_match_visitor (match_ctx_t *ctx, visitor_ctx->command_name = bson_strdup (bson_iter_key (doc_iter)); } - // Subdocuments in `command` are expected not to not have extra fields. + // Subdocuments in `command` should not have extra fields. if (NULL != strstr (ctx->path, ".command") && doc_iter) { if (BSON_ITER_HOLDS_DOCUMENT (doc_iter) && BSON_ITER_HOLDS_DOCUMENT (pattern_iter)) { @@ -663,7 +663,7 @@ test_apm_matching (void) static void test_apm_matching_extra_fields (void) { - // An extra value in `command` is permitted. + // Extra fields are permitted in `command`. { apm_match_visitor_ctx_t match_visitor_ctx = {0}; match_ctx_t match_ctx = {{0}}; @@ -682,7 +682,7 @@ test_apm_matching_extra_fields (void) apm_match_visitor_ctx_reset (&match_visitor_ctx); } - // A subdocument in `command` does not permit extra values. + // Extra fields are not permitted in `command` sub-documents. { apm_match_visitor_ctx_t match_visitor_ctx = {0}; match_ctx_t match_ctx = {{0}}; @@ -703,7 +703,7 @@ test_apm_matching_extra_fields (void) apm_match_visitor_ctx_reset (&match_visitor_ctx); } - // A subdocument in a subarray in `command` does not permit extra values. + // Extra fields are not permitted in `command` sub-arrays. { apm_match_visitor_ctx_t match_visitor_ctx = {0}; match_ctx_t match_ctx = {{0}}; From 546387ec13358a461162a76e304fa8ccaefffdb6 Mon Sep 17 00:00:00 2001 From: Kevin Albertson Date: Thu, 8 Jun 2023 15:45:02 -0400 Subject: [PATCH 7/8] document handling of visitor_fn return --- src/libmongoc/tests/test-conveniences.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libmongoc/tests/test-conveniences.c b/src/libmongoc/tests/test-conveniences.c index a74154f7e97..f70bdfb84bc 100644 --- a/src/libmongoc/tests/test-conveniences.c +++ b/src/libmongoc/tests/test-conveniences.c @@ -977,10 +977,14 @@ match_bson_with_ctx (const bson_t *doc, const bson_t *pattern, match_ctx_t *ctx) match_action_t action = ctx->visitor_fn (ctx, &pattern_iter, found ? &doc_iter : NULL); if (action == MATCH_ACTION_ABORT) { + // Visitor encountered a match error. goto fail; } else if (action == MATCH_ACTION_SKIP) { + // Visitor handled match of this field. Skip any additional matching + // of this field. goto next; } + ASSERT (action == MATCH_ACTION_CONTINUE); } if (value->value_type == BSON_TYPE_NULL && found) { @@ -1306,10 +1310,14 @@ match_bson_arrays (const bson_t *array, const bson_t *pattern, match_ctx_t *ctx) match_action_t action = ctx->visitor_fn (ctx, &pattern_iter, &array_iter); if (action == MATCH_ACTION_ABORT) { + // Visitor encountered a match error. return false; } else if (action == MATCH_ACTION_SKIP) { + // Visitor handled match of this field. Skip any additional matching + // of this field. continue; } + ASSERT (action == MATCH_ACTION_CONTINUE); } if (!match_bson_value (array_value, pattern_value, &derived)) { From 67b4f7e807912f0675886fbde97cfb1d5ae48410 Mon Sep 17 00:00:00 2001 From: Kevin Albertson Date: Thu, 8 Jun 2023 15:47:24 -0400 Subject: [PATCH 8/8] remove unnecessary `void*` cast --- src/libmongoc/tests/json-test-monitoring.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libmongoc/tests/json-test-monitoring.c b/src/libmongoc/tests/json-test-monitoring.c index b27b6de2411..33ae0a28d46 100644 --- a/src/libmongoc/tests/json-test-monitoring.c +++ b/src/libmongoc/tests/json-test-monitoring.c @@ -554,7 +554,7 @@ check_json_apm_events (json_test_ctx_t *ctx, const bson_t *expectations) match_ctx.retain_dots_in_keys = true; match_ctx.allow_placeholders = true; match_ctx.visitor_fn = apm_match_visitor; - match_ctx.visitor_ctx = (void *) &apm_match_visitor_ctx; + match_ctx.visitor_ctx = &apm_match_visitor_ctx; allow_subset = ctx->config->command_monitoring_allow_subset; @@ -649,7 +649,7 @@ test_apm_matching (void) "}"; match_ctx.visitor_fn = apm_match_visitor; - match_ctx.visitor_ctx = (void *) &match_visitor_ctx; + match_ctx.visitor_ctx = &match_visitor_ctx; BSON_ASSERT (match_bson_with_ctx (tmp_bson (e1), tmp_bson (e1), &match_ctx)); BSON_ASSERT ( @@ -674,7 +674,7 @@ test_apm_matching_extra_fields (void) BSON_STR ({"command_started_event" : {"command" : {"a" : 1}}}); match_ctx.visitor_fn = apm_match_visitor; - match_ctx.visitor_ctx = (void *) &match_visitor_ctx; + match_ctx.visitor_ctx = &match_visitor_ctx; bool matched = match_bson_with_ctx (tmp_bson (event), tmp_bson (pattern), &match_ctx); @@ -694,7 +694,7 @@ test_apm_matching_extra_fields (void) {"command_started_event" : {"command" : {"subdoc" : {"a" : 1}}}}); match_ctx.visitor_fn = apm_match_visitor; - match_ctx.visitor_ctx = (void *) &match_visitor_ctx; + match_ctx.visitor_ctx = &match_visitor_ctx; bool matched = match_bson_with_ctx (tmp_bson (event), tmp_bson (pattern), &match_ctx); @@ -717,7 +717,7 @@ test_apm_matching_extra_fields (void) }); match_ctx.visitor_fn = apm_match_visitor; - match_ctx.visitor_ctx = (void *) &match_visitor_ctx; + match_ctx.visitor_ctx = &match_visitor_ctx; bool matched = match_bson_with_ctx (tmp_bson (event), tmp_bson (pattern), &match_ctx);