Skip to content

CDRIVER-4489 promote empty username to client error #1939

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.

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 5 commits into from
Mar 20, 2025

Conversation

eramongodb
Copy link
Contributor

Followup to #1896. Enforces that even optional usernames are non-empty when specified (there are no cases where an empty username is desirable).

@eramongodb eramongodb requested a review from kevinAlbs March 19, 2025 20:12
@eramongodb eramongodb self-assigned this Mar 19, 2025
Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

LGTM with minor comments addressed.

BSON_ASSERT_PARAM (mechanism);
BSON_ASSERT_PARAM (user_prefix);

bson_error_t error;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move declaration inside scope block to fix warnings on Evergreen tasks:

../../../src/libmongoc/tests/test-mongoc-uri.c: In function '_auth_mechanism_password_prohibited':
../../../src/libmongoc/tests/test-mongoc-uri.c:339:20: error: declaration of 'error' shadows a previous local [-Werror=shadow]
  339 |       bson_error_t error;
      |                    ^~~~~

Comment on lines 326 to 327
mongoc_uri_t *const uri = mongoc_uri_new_with_error (
tmp_str ("mongodb://localhost/?" MONGOC_URI_AUTHMECHANISM "=%s%s", mechanism, uri_suffix), &error);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
mongoc_uri_t *const uri = mongoc_uri_new_with_error (
tmp_str ("mongodb://localhost/?" MONGOC_URI_AUTHMECHANISM "=%s%s", mechanism, uri_suffix), &error);
mongoc_uri_t *const uri = mongoc_uri_new_with_error (
tmp_str ("mongodb://%s@localhost/?" MONGOC_URI_AUTHMECHANISM "=%s%s", user_prefix, mechanism, uri_suffix),
&error);

Suggest including user_prefix. IIUC this helper only intends to test changes in the password and keep the rest of the URI fixed.

If accepted, also update or remove the ASSERT_CMPSTR (mongoc_uri_get_username (uri), NULL); below.

@eramongodb eramongodb merged commit b1ff5d6 into mongodb:master Mar 20, 2025
1 check was pending
@eramongodb eramongodb deleted the cdriver-4489-pre branch March 20, 2025 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants