DiscussionParser: Don't construct Users with invalid names

The comment said "don't validate the username - anon (IP) is fine!"
– but it also allowed invalid names, not just IPs, and those are not
really fine.

Also add more test cases and remove some unused test code.

Bug: T380242
Change-Id: Id98f14a0663f33eb5e45045bcd2df6a1e1f52de6
This commit is contained in:
Bartosz Dziewoński 2024-11-19 23:14:35 +01:00
parent 09daeb5ba2
commit 124162fee4
2 changed files with 34 additions and 11 deletions

View file

@ -1117,6 +1117,8 @@ abstract class DiscussionParser {
*/
public static function getUserFromLine( $line, ?Title $title = null ) {
$parser = MediaWikiServices::getInstance()->getParser();
$userNameUtils = MediaWikiServices::getInstance()->getUserNameUtils();
$userFactory = MediaWikiServices::getInstance()->getUserFactory();
/*
* First we call extractUsersFromLine to get all the potential usernames
@ -1129,8 +1131,15 @@ abstract class DiscussionParser {
foreach ( $usernames as $username ) {
// generate (dateless) signature from the user we think we've
// discovered the signature from
// don't validate the username - anon (IP) is fine!
$user = User::newFromName( $username, false );
if ( $userNameUtils->isIP( $username ) ) {
$user = $userFactory->newAnonymous( $username );
} else {
$user = $userFactory->newFromName( $username );
if ( !$user ) {
// Invalid username, so this link can't be any user's signature
continue;
}
}
$sig = $parser->preSaveTransform(
'~~~',
$title ?: Title::newMainPage(),

View file

@ -1047,15 +1047,7 @@ TEXT
$output = DiscussionParser::getUserFromLine( $line );
if ( $output === false ) {
$this->assertFalse( $expectedUser );
} elseif ( is_array( $expectedUser ) ) {
// Sometimes testing for correct user detection,
// sometimes testing for offset detection
$this->assertEquals( $expectedUser, $output );
} else {
$this->assertEquals( $expectedUser, $output[1] );
}
}
public static function signingDetectionDataProvider() {
@ -1090,10 +1082,32 @@ TEXT
'127.0.0.1'
],
],
"Anonymous user (not a standard signature - userpage link)" => [
"I am anonymous because I like my IP address. --[[User:127.0.0.1|127.0.0.1]] $ts",
false,
],
"No signature" => [
"Well, \nI do think that [[User:Newyorkbrad]] is pretty cool, but what do I know?",
false
],
"Invalid username link" => [
"I'm evil! [[User:Template:Invalid|Invalid]] $ts",
false
],
"Invalid username link, followed by a valid one" => [
"I'm silly! --[[User:JarJar|JarJar]] ([[User talk:JarJar|talk]]) [[User:Template:Invalid|Invalid]] $ts",
[
13,
'JarJar'
]
],
"Invalid username link, preceded by a valid one" => [
"I'm silly! [[User:Template:Invalid|Invalid]] --[[User:JarJar|JarJar]] ([[User talk:JarJar|talk]]) $ts",
[
13 + 34,
'JarJar'
]
],
"Hash symbols in usernames" => [
"What do you think? [[User talk:We buried our secrets in the garden#top|wbositg]] $ts",
[