[3.0] Keeps a reported profile's name inside the column that stores it - #9618
Open
albertlast wants to merge 1 commit into
Open
[3.0] Keeps a reported profile's name inside the column that stores it#9618albertlast wants to merge 1 commit into
albertlast wants to merge 1 commit into
Conversation
Reporting a profile labels the report with the member's display name and their username in brackets after it. Both names come out of the database entity encoded, and both are wide enough on their own that the pair does not necessarily fit the 255 characters log_reported.membername holds: real_name is 255 wide and member_name 80. The display name gets there because the profile validator counts the characters that were typed while the column stores the encoded form of them. A display name of 42 double quotes is 42 characters to the validator, well inside the limit of 60, and 252 characters in the column. Reporting that profile builds a 259 character label, and the insert fails with "Data too long for column 'membername'". Adds the username only when there is room for it. The display name is what identifies the member, and it came out of a column the same width, so it always fits on its own. 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
Found while working on #9616, and kept separate from it because it is a different
column, a different code path and a different cause.
Reporting a profile labels the report with the member's display name and their
username in brackets after it:
That goes into
log_reported.membername, which isvarchar(255). Butmembers.real_nameis 255 wide on its own andmembers.member_nameis 80, so thepair has no guarantee of fitting. When it does not, the insert fails with
How a display name gets long enough
real_nameis limited to 60 characters, but the limit is counted on the charactersthat were typed while the column stores the entity encoded form of them:
Sources/Actions/Profile/Main.phprunsUtils::htmlspecialcharsRecursive($_POST, ENT_QUOTES)before validation, so by the time the value is checked a double quote is already
"— six characters.Utils::entityStrlen($value) > 60, which decodes first, so itsees the six characters as one again.
A display name of 42 double quotes is therefore 42 characters to the validator, well
inside the limit, and 252 characters in the column, inside
varchar(255). Verifiedagainst the real methods:
Any username at all then pushes the label past 255 and the report cannot be filed.
Are those characters actually allowed in a name?
In a username, no.
Security::validateUsername()rejects<>&"'=\outright. But thatrule is only applied to
member_name. The display name has its own validator, and theonly character it forbids is
*, inSecurity::isReservedName().Running that validator against a real database, as the admin, with the value encoded the
way
Profile\Mainencodes it before validation:So the display name is the vector, and
*being the one rejection confirms the check isthe real gate rather than a no-op.
This is adversarial rather than accidental — you do not reach it by having a long
name, you reach it by having a name made mostly of characters that need entities. It
is still a display name a member is allowed to set, and the failure lands on whoever
tries to report them, which is exactly the wrong person to punish for it.
The fix
Add the username only when there is room for it. The display name is what identifies
the member, it is what the moderation centre shows, and it came out of a column the
same width as this one, so on its own it always fits.
membernameis only read as the fallback for a member who no longer exists —ReportedContentselectsCOALESCE(mem.real_name, lr.membername)— so dropping thebracketed username in this case costs very little, and costs it only for names that
would otherwise make the report impossible to file.
Measuring with
mb_strlen()rather thanUtils::entityStrlen()is deliberate: bothnames arrive from the database already encoded, so the encoded length is what the
column has to hold.
Config.phpusesmb_strlen()the same way for the same kind of"does this fit" question.
Verification
composer lintandvendor/bin/phpunit(280 tests, 495 assertions) pass.No unit test:
ReportToMod::reportMember()queries the database on its first line andUtils::htmlspecialchars()is the only part of the path reachable from the suite. Thenumbers above were produced by running the real
Utilsmethods in the Dockerenvironment.
Issues References (Fixes|Related|Closes)
🤖 Generated with Claude Code