MDEV-39245-Check that plugin name is pure ASCII#4950
Draft
pranavktiwari wants to merge 1 commit into10.11from
Draft
MDEV-39245-Check that plugin name is pure ASCII#4950pranavktiwari wants to merge 1 commit into10.11from
pranavktiwari wants to merge 1 commit into10.11from
Conversation
pranavktiwari
force-pushed
the
10.11-MDEV-39245
branch
2 times, most recently
from
3649181 to
2fab82e
Compare
pranavktiwari
force-pushed
the
10.11-MDEV-39245
branch
3 times, most recently
from
bba9d91 to
4b5e148
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses MDEV-39245 by enforcing that server plugin names are pure ASCII at registration time, preventing undefined behavior caused by non-ASCII names being handled with ASCII/latin1-oriented routines.
Changes:
- Added ASCII repertoire validation for plugin names during dynamic plugin registration (plugin_add) and built-in plugin registration (plugin_init).
- Added new test plugins (one all-ASCII, one containing a non-ASCII plugin name) plus an MTR test verifying failure/success cases.
- Updated Debian test packaging to include the new test plugin .so files.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| sql/sql_plugin.cc | Adds ASCII-only validation for plugin names during plugin registration/initialization. |
| plugin/plugin_name_ascii_check/CMakeLists.txt | Builds two new test plugin modules used by MTR. |
| plugin/plugin_name_ascii_check/plugin_name_ascii.cc | Test plugin with only ASCII plugin names. |
| plugin/plugin_name_ascii_check/plugin_name_non_ascii.cc | Test plugin that includes a non-ASCII plugin name to validate rejection behavior. |
| mysql-test/suite/plugins/t/plugin_name_ascii.test | New MTR test covering INSTALL SONAME / INSTALL PLUGIN scenarios for ASCII vs non-ASCII names. |
| mysql-test/suite/plugins/t/plugin_name_ascii.opt | Test options for plugin maturity. |
| mysql-test/suite/plugins/r/plugin_name_ascii.result | Expected output for the new MTR test. |
| debian/mariadb-test.install | Installs the new test plugin shared objects for Debian test packages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+17
to
+20
| Test plugin: two plugins with ASCII names and one with a non-ASCII name. | ||
| Used by mysql-test/suite/plugins/t/plugin_name_ascii.test | ||
| The non-ASCII plugin name "plügïn_bad" is represented as UTF-8 hex bytes. | ||
| */ |
| { | ||
| MYSQL_DAEMON_PLUGIN, | ||
| &plugin3_descriptor, | ||
| "plügïn_non_ascii", /* "plügïn_non_ascii" in UTF-8 bytes */ |
Comment on lines
+1152
to
+1156
| /* Pre-validation: ensure all plugin names are pure ASCII */ | ||
| for (plugin= tmp.plugin_dl->plugins; plugin->info; plugin++) | ||
| { | ||
| tmp.name.str= (char *)plugin->name; | ||
| tmp.name.length= strlen(plugin->name); | ||
| if (skip_plugin(plugin)) | ||
| continue; |
Author
There was a problem hiding this comment.
it is possible that name -> str is null- in that case all the plugins from .so should be installed only if all of them contains ascii plugin name.
But if name is passed from the argument, in that case only the name with that plugin needs to be installed only if the name is ascii character. What is wrong in that?
| tmp.name.length)) | ||
| { | ||
| print_init_failed_error(&tmp); | ||
| DBUG_RETURN(1);; |
pranavktiwari
force-pushed
the
10.11-MDEV-39245
branch
2 times, most recently
from
5829cc2 to
1194e8f
Compare
sanja-byelkin
requested changes
Apr 23, 2026
| plugin->name, strlen(plugin->name))) | ||
| { | ||
| my_error(ER_PLUGIN_IS_NOT_LOADED, MyFlags, plugin->name, ENOEXEC, | ||
| "invalid plugin name: non-ASCII characters are not supported"); |
Member
There was a problem hiding this comment.
where this text goes? have you checked the pattern? have you wrote it or AI?
tip: use my_printf_error
| FROM information_schema.PLUGINS | ||
| WHERE PLUGIN_NAME = 'ascii_plugin_one'; | ||
|
|
||
| UNINSTALL PLUGIN ascii_plugin_one; No newline at end of file |
Member
There was a problem hiding this comment.
Please correct line ends everywhere (setup your editor)
pranavktiwari
force-pushed
the
10.11-MDEV-39245
branch
from
1194e8f to
abf87e1
Compare
1. Built-in plugins (plugin_init): A non-ASCII builtin name is a developer error. Added a check using my_string_repertoire() that prints an error via print_init_failed_error() and returns 1 to abort server startup. 2. Dynamically loaded plugins (plugin_add): Added a pre-validation loop before the registration loop. If any plugin name in the .so is non-ASCII, the entire .so is rejected before any plugin is registered, preventing partial state where valid plugins from the same .so get left INACTIVE in the hash. Added test plugins and MTR tests to verify both rejection of non-ASCII named plugins and successful loading of ASCII named plugins.
pranavktiwari
force-pushed
the
10.11-MDEV-39245
branch
from
abf87e1 to
e42638f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes MDEV-39245
Problem:
It is assumed plugin names are pure ASCII (e.g., uses my_casedn_str with latin1 charset) but never validates this at plugin registration time, allowing plugins with non-ASCII names to be installed, causing undefined behavior.
Fix
Added ASCII validation at two points using my_string_repertoire():
Built-in plugins (plugin_init):
A non-ASCII builtin name is a developer error. Added a check
using my_string_repertoire() that prints an error via
print_init_failed_error() and returns 1 to abort server startup.
Dynamically loaded plugins (plugin_add):
Added a pre-validation loop before the registration loop. If any
plugin name in the .so is non-ASCII, the entire .so is rejected
before any plugin is registered, preventing partial state where
valid plugins from the same .so get left INACTIVE in the hash.
Added test plugins and MTR tests to verify both rejection of
non-ASCII named plugins and successful loading of ASCII named plugins.