MDEV-41186 Fix stack-buffer-overflow in make_unique_constraint_name

[sanja-byelkin]

A multi-byte PERIOD name at the 64-character limit (192 bytes) filled the name buffer exactly, leaving no room for the '_N' suffix appended when generating a unique name for the implicit CHECK constraint.

Truncate on a character boundary, but only once a suffix is actually needed (mirroring make_unique_key_name), so a non-colliding name is never altered.

[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.}

[vaintroub]

The truncation budget in make_unique_constraint_name() is byte-based, not character-based, so it doesn't fully fix the underlying issue for all-ASCII names.

size_t avail= sizeof(buff) - (1 + MY_INT32_NUM_DECIMAL_DIGITS + 1);
end= buff + Well_formed_prefix(system_charset_info, buff,
                               MY_MIN(size_t(end - buff), avail))
             .length();
*end++= '_';

avail (180 bytes) is well under the 193-byte buffer, which is enough to prevent the overflow this patch targets. But the actual identifier limit, NAME_CHAR_LEN, is 64 characters, not bytes. For a 64-character all-ASCII base name (64 bytes), MY_MIN(64, 180) == 64, so Well_formed_prefix truncates nothing, and the _1 suffix is appended anyway — producing a 66-character internal constraint name that exceeds NAME_CHAR_LEN.

I reproduced this by building the branch and running it live:

CREATE TABLE t2 (s DATE NOT NULL, e DATE NOT NULL,
                 PERIOD FOR xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (s, e),
                 CONSTRAINT xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx CHECK (s < e));
ALTER TABLE t2 DROP CONSTRAINT `xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx_1`;

ERROR 4158 (HY000): Can't DROP CONSTRAINT `xxxx…xx_1`. Use DROP PERIOD `xxxx…xx` for this

That error proves a genuine 66-character constraint name ("x" * 64 + "_1") was created and stored — it isn't just a display artifact. It looks invisible in information_schema.CHECK_CONSTRAINTS only because CONSTRAINT_NAME there is VARCHAR(NAME_CHAR_LEN) (64) and silently truncates it back down, which then makes it display as a byte-for-byte duplicate of the explicit constraint's name.

This is worth fixing in this same patch, since Well_formed_prefix already has a 3-arg overload that caps by character count directly:

Well_formed_prefix(CHARSET_INFO *cs, const char *str, size_t length, size_t nchars)

Suggested fix — cap by characters as well as bytes:

size_t avail= sizeof(buff) - (1 + MY_INT32_NUM_DECIMAL_DIGITS + 1);
size_t avail_chars= NAME_CHAR_LEN - (1 + MY_INT32_NUM_DECIMAL_DIGITS);
end= buff + Well_formed_prefix(system_charset_info, buff,
                               MY_MIN(size_t(end - buff), avail),
                               avail_chars)
             .length();
*end++= '_';

Test case that fails against the current patch (add alongside the existing multi-byte one in mysql-test/suite/period/t/create.test):

--echo # 64-character all-ASCII period name, same collision as above
CREATE TABLE t2 (s DATE NOT NULL, e DATE NOT NULL,
                 PERIOD FOR xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (s, e),
                 CONSTRAINT xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx CHECK (s < e));
--error ER_PERIOD_CONSTRAINT_DROP
ALTER TABLE t2 DROP CONSTRAINT `xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx_1`;
DROP TABLE t2;

[vaintroub]

Claude found a bug that can produce duplicate constraint names, using just ASCII long names, as in comment.
Could you please fix it.

Otherwise it looks good.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Suffixed names can still exceed the 64-character identifier limit for ASCII or shorter multibyte PERIOD names.

Review effort: Lite
Findings: None

What changed in this PR

Fixes a stack-buffer overflow when generating unique implicit CHECK constraint names for maximum-length multibyte PERIOD identifiers.

Changes:

• Truncates names at character boundaries when suffixes are required.
• Adds regression coverage for collision and non-collision cases.

File	Changes
sql/​sql_table.cc	Adjusts unique constraint name generation.
mysql-test/​suite/​period/​t/​create.test	Adds regression scenarios.
mysql-test/​suite/​period/​r/​create.result	Records expected results.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[vaintroub]

Approved — this fixes the byte-vs-character gap correctly, and I rebuilt and ran period.create against this head, it passes.

One small cleanup suggestion on the new ASCII test case: the ALTER TABLE t2 DROP CONSTRAINT xxxx…xx_1; --error ER_PERIOD_CONSTRAINT_DROP step can be removed. It was useful on the previous commit to prove the over-long (66-char) name existed despite being invisible in information_schema.CHECK_CONSTRAINTS (hence needing the DROP-error trick to surface it). Now that the generated name is a full 10 characters shorter than the limit, there's no more ambiguity between the period name and the generated constraint name, and the preceding SELECT CONSTRAINT_NAME, CHAR_LENGTH(...), LENGTH(...) already shows the real, complete, in-bounds name directly — so the DROP CONSTRAINT step doesn't add coverage for this bug, and it hardcodes a name that depends on the exact truncation arithmetic (NAME_CHAR_LEN - (1 + MY_INT32_NUM_DECIMAL_DIGITS)), so it'll be a nuisance to keep in sync if that budget ever shifts for unrelated reasons.

[vaintroub]

Approved, but with suggestion to drop failing DROP CONSTRAINT from test now, since it was only necessary to show ambiguity previously.