[3.0] Measures a display name against the column that has to hold it - #9619
Open
albertlast wants to merge 1 commit into
Open
[3.0] Measures a display name against the column that has to hold it#9619albertlast wants to merge 1 commit into
albertlast wants to merge 1 commit into
Conversation
The display name limit is 60 characters, counted with entityStrlen() on a value that has already been entity encoded, so it counts what was typed. What real_name stores is the encoded form, where a double quote is six characters and an ampersand five, and that column holds 255. A name of 43 double quotes is therefore 43 characters to the validator, comfortably inside the limit, and 258 in the column. Saving it dies with "Data too long for column 'real_name'" out of User::updateMemberData(). Registration has the same gap from the other direction: it checks the raw value against 60 and encodes it afterwards. Checks the encoded width alongside the typed length in both places, so a name that cannot be stored is refused by the form that offered it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
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.
Description
The third of the same mistake, found while working on #9616 and #9618: a length limit
counted in typed characters guarding a column that stores the entity encoded form of
them.
The display name limit is 60 characters. In the profile it is enforced by
Sources/Actions/Profile/Main.phprunsUtils::htmlspecialcharsRecursive($_POST, ENT_QUOTES)before validation, so
$valueis already encoded when it gets here, andentityStrlen()decodes it again to count. That is the right number to show a member. It is not the number
members.real_namehas to hold, which is the encoded one — six characters per doublequote, five per ampersand, four per angle bracket — in a
varchar(255).What happens
A display name of 43 double quotes is 43 characters to the validator and 258 in the
column. Run against the real validator and
User::updateMemberData()on the Docker stack:The profile save dies on it. PostgreSQL gives
value too long for type character varying(255)in the same place.These characters really are allowed in a display name, which is the part that is easy to
get wrong:
Security::validateUsername()rejects<>&"'=\, but that rule applies only tomember_name. The display name's validator forbids exactly one character,*, inSecurity::isReservedName().Registration has the same gap coming the other way.
Register2checks the raw valueand then encodes it thirty lines later when building
extra_register_vars, so the valuethat is measured and the value that is stored are again not the same string.
The fix
Check the encoded width alongside the typed length, in both places, so a name that cannot
be stored is refused by the form that offered it rather than by the database.
mb_strlen()rather thanUtils::entityStrlen()is the point of the change: the encodedlength is what the column has to hold, and measuring it with
entityStrlen()is the bug.Why not widen the column instead
That is what #9616 does for the report comment, and it is the nicer answer where it is
available, because the limit the member is shown then means what it says. It is not
available here:
real_namecarries three indexes —idx_real_name,idx_active_real_name, andidx_real_name_lowon PostgreSQL — so it cannot become atextcolumn on MySQL without prefix lengths, and widening amemberscolumn that threeindexes and a great deal of code depend on is a much larger change than this bug warrants.
The cost of doing it this way is that a name of 43 double quotes is refused as "too long"
while the message says the limit is 60. That is a poor explanation, but it is a poor
explanation of a refusal rather than a fatal error, and it is only reachable by a name
made mostly of characters that need entities. I have deliberately left
$txt['profile_error_name_too_long']alone rather than reword it and stale everytranslation for this case; happy to change that if you would rather.
Verification
Against the patched validator, on a real database:
An ordinary 60 character name is still accepted, the widest name that actually fits is
still accepted, the one that used to fatal is now refused, and the original 60 character
limit still works.
composer lintandvendor/bin/phpunit(280 tests, 495 assertions) pass.No unit test: the validator is a closure inside
Profile::loadStandardFields(), whichneeds a loaded member and therefore a database, and
Register2reads$_POSTand queriespermissions. The numbers above came from running the real code against MySQL instead.
Issues References (Fixes|Related|Closes)
🤖 Generated with Claude Code