MDEV-38862: server_audit: Fix double comma in user lists causing incorrect filtering

[KevinDryden]

When SERVER_AUDIT_INCL_USERS or SERVER_AUDIT_EXCL_USERS contains double commas (e.g., 'user1,,user2'), the audit plugin behaves incorrectly:

• For incl_users: ALL users are logged instead of only specified users
• For excl_users: ALL users are excluded instead of only specified users

The root cause is in user_coll_fill(). When parsing a user list string with consecutive commas, the parser calls getkey_user() with the pointer positioned at a comma, which returns cmp_length of 0. Then coll_insert() inserts an empty string into the user collection, corrupting the collection's search behavior.

The fix adds a check to skip empty tokens (when the current character is a comma after whitespace has been skipped) before attempting to extract a username.

Testing

Added 6 MTR tests to verify the fix handles all edge cases:

Test	Description
plugins.server_audit_double_comma	Double comma in incl_users ('user1,,user2')
plugins.server_audit_excl_double_comma	Double comma in excl_users
plugins.server_audit_edge_commas	Leading/trailing commas (',user1,user2,')
plugins.server_audit_empty_input	Empty and whitespace-only input
plugins.server_audit_multiple_commas	Multiple consecutive commas ('user1,,,,,user2')
plugins.server_audit_whitespace	Whitespace around usernames (' user1 , user2 ')

Test Results - With Fix (MariaDB main branch)

TEST                                      RESULT   TIME (ms)
--------------------------------------------------------------------------
plugins.server_audit_double_comma        [ pass ]     10
plugins.server_audit_edge_commas         [ pass ]     22
plugins.server_audit_empty_input         [ pass ]     18
plugins.server_audit_excl_double_comma   [ pass ]     10
plugins.server_audit_multiple_commas     [ pass ]     10
plugins.server_audit_whitespace          [ pass ]     15
--------------------------------------------------------------------------
Completed: All 6 tests were successful.

Test Results - Without Fix (Bug Reproduction)

To verify the tests correctly detect the bug, we reverted the fix and ran the tests:

TEST                                      RESULT   TIME (ms)
--------------------------------------------------------------------------
plugins.server_audit_double_comma        [ fail ]
plugins.server_audit_edge_commas         [ fail ]
plugins.server_audit_empty_input         [ pass ]     18
plugins.server_audit_excl_double_comma   [ fail ]
plugins.server_audit_multiple_commas     [ fail ]
plugins.server_audit_whitespace          [ fail ]
--------------------------------------------------------------------------
Failed 5/6 tests, 16.67% were successful.

Note: server_audit_empty_input passes without the fix because it tests different behavior - when incl_users contains only commas/whitespace (no valid usernames), the expected behavior is to log ALL users (empty inclusion list = no filtering). This is correct behavior that works with or without the fix. The other 5 tests verify the bug scenario where valid usernames are mixed with empty tokens.

All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[gkodinov]

Thank you for your contribution! This is a preliminary review.
Please clear the CLA bot: click on the CLA button and make sure you sign the CLA.

Then please re-base to 10.6. This is a bug and is reproducible in 10.6 so let's please fix it as per the guideline.

[gkodinov]

Why not just call getkey_user and check if cmp_length is zero? That's how you're describing the fix. This has the advantage that it will also cover empty user at the end, i.e. a trailing comma.

[gkodinov]

why no view protocol?

[gkodinov]

All of these tests are essentially the same test, just with different parameters and different expectations.
Why not have the bulk of these in an inc file and just pass parameters to it?

[gkodinov]

It's been more than 3 weeks since I've last replied to this PR. And I've got no response. Closing it due to inactivity. Please re-open if/when you intend to continue working on it.