Skip to content

Commit

Permalink
MONGOCRYPT-576 Include the error from mcr_dll_open in hard error (#638
Browse files Browse the repository at this point in the history
)

* add a failing test

* add optional status to _try_load_csfle
  • Loading branch information
kevinAlbs authored May 7, 2023
1 parent 1525897 commit 792beee
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 6 deletions.
17 changes: 11 additions & 6 deletions src/mongocrypt.c
Original file line number Diff line number Diff line change
Expand Up @@ -362,8 +362,10 @@ typedef struct {
/**
* @brief Attempt to open the CSFLE dynamic library and initialize a vtable for
* it.
*
* @param status is an optional status to set an error message if `mcr_dll_open` fails.
*/
static _loaded_csfle _try_load_csfle(const char *filepath, _mongocrypt_log_t *log) {
static _loaded_csfle _try_load_csfle(const char *filepath, _mongocrypt_log_t *log, mongocrypt_status_t *status) {
// Try to open the dynamic lib
mcr_dll lib = mcr_dll_open(filepath);
// Check for errors, which are represented by strings
Expand All @@ -374,6 +376,7 @@ static _loaded_csfle _try_load_csfle(const char *filepath, _mongocrypt_log_t *lo
"Error while opening candidate for CSFLE dynamic library [%s]: %s",
filepath,
lib.error_string.data);
CLIENT_ERR("Error while opening candidate for CSFLE dynamic library [%s]: %s", filepath, lib.error_string.data);
// Free resources, which will include the error string
mcr_dll_close(lib);
// Bad:
Expand Down Expand Up @@ -476,7 +479,7 @@ static _loaded_csfle _try_find_csfle(mongocrypt_t *crypt) {
// Do not allow a plain filename to go through, as that will cause the
// DLL load to search the system.
mstr_assign(&csfle_cand_filepath, mpath_absolute(csfle_cand_filepath.view, MPATH_NATIVE));
candidate_csfle = _try_load_csfle(csfle_cand_filepath.data, &crypt->log);
candidate_csfle = _try_load_csfle(csfle_cand_filepath.data, &crypt->log, crypt->status);
}
} else {
// No override path was specified, so try to find it on the provided
Expand All @@ -498,7 +501,7 @@ static _loaded_csfle _try_find_csfle(mongocrypt_t *crypt) {
}
}
// Try to load the file:
candidate_csfle = _try_load_csfle(csfle_cand_filepath.data, &crypt->log);
candidate_csfle = _try_load_csfle(csfle_cand_filepath.data, &crypt->log, NULL /* status */);
if (candidate_csfle.okay) {
// Stop searching:
break;
Expand Down Expand Up @@ -817,9 +820,11 @@ static bool _try_enable_csfle(mongocrypt_t *crypt) {
// If a crypt_shared override path was specified, but we did not succeed in
// loading crypt_shared, that is a hard-error.
if (crypt->opts.crypt_shared_lib_override_path.data && !found.okay) {
CLIENT_ERR("A crypt_shared override path was specified [%s], but we failed to "
"open a dynamic library at that location",
crypt->opts.crypt_shared_lib_override_path.data);
// Wrap error with additional information.
CLIENT_ERR("A crypt_shared override path was specified [%s], but we failed to open a dynamic "
"library at that location. Load error: [%s]",
crypt->opts.crypt_shared_lib_override_path.data,
mongocrypt_status_message(crypt->status, NULL /* len */));
return false;
}

Expand Down
13 changes: 13 additions & 0 deletions test/test-mongocrypt-csfle-lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,18 @@ static void _test_csfle_not_loaded_with_bypassqueryanalysis(_mongocrypt_tester_t
mongocrypt_destroy(crypt);
}

// _test_override_error_includes_reason test changes of MONGOCRYPT-576: the error message from mcr_dll_open is
// propagated.
static void _test_override_error_includes_reason(_mongocrypt_tester_t *tester) {
mongocrypt_t *crypt = get_test_mongocrypt(tester);
// Set an incorrect override path.
mongocrypt_setopt_set_crypt_shared_lib_path_override(crypt, "invalid_path_to_crypt_shared.so");
ASSERT_FAILS(mongocrypt_init(crypt), crypt, "Error while opening candidate");
BSON_ASSERT(mongocrypt_crypt_shared_lib_version_string(crypt, NULL) == NULL);
BSON_ASSERT(mongocrypt_crypt_shared_lib_version(crypt) == 0);
mongocrypt_destroy(crypt);
}

void _mongocrypt_tester_install_csfle_lib(_mongocrypt_tester_t *tester) {
INSTALL_TEST(_test_csfle_no_paths);
INSTALL_TEST(_test_csfle_not_found);
Expand All @@ -168,4 +180,5 @@ void _mongocrypt_tester_install_csfle_lib(_mongocrypt_tester_t *tester) {
INSTALL_TEST(_test_csfle_path_override_fail);
INSTALL_TEST(_test_cur_exe_path);
INSTALL_TEST(_test_csfle_not_loaded_with_bypassqueryanalysis);
INSTALL_TEST(_test_override_error_includes_reason);
}

0 comments on commit 792beee

Please sign in to comment.