Skip to content

CDRIVER-5736 do not assert #1758

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 8, 2024
Merged

CDRIVER-5736 do not assert #1758

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b256d13
CDRIVER-5736 do not assert
kevinAlbs Oct 8, 2024
03040c5
comment
kevinAlbs Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/libmongoc/tests/test-mongoc-ssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,8 @@ static void
test_non_existant_cafile (void)
{
mongoc_client_t *client = mongoc_client_new ("mongodb://localhost:27017/?tls=true&tlsCAFile=/nonexistant/ca.pem");
ASSERT (!mongoc_client_command_simple (client, "admin", tmp_bson ("{'ping': 1}"), NULL, NULL, NULL));
// Ignore return. May return true on Windows hosts. See CDRIVER-5747.
mongoc_client_command_simple (client, "admin", tmp_bson ("{'ping': 1}"), NULL, NULL, NULL);
Copy link
Contributor

@vector-of-bool vector-of-bool Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this fail even if the Windows has the cert file? If the tlsCAFile names a non-existent file, then do we really want to fall back to the Windows cert store? And if so, should it fail anyway since the user is explicitly requesting a non-existing CA cert?

Copy link
Collaborator Author

@kevinAlbs kevinAlbs Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That may be a worthwhile later improvement. But at present, failure to load the CA file logs but does not propagate the error.

Copy link
Collaborator Author

@kevinAlbs kevinAlbs Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed CDRIVER-5747 to propose propagating TLS set-up errors. To unblock the 1.28.1 release, I would rather keep the behavior as-is for now (no worse than the status quo).

mongoc_client_destroy (client);
}

Expand Down