This solves two issues described in bug T360909:
* Usage of unsafe characters that have to be
manually reviewed in translations.
* Incorect display of some functions and
operators in RTL UI languages.
It also reduces the translators' need to copy
those operators and functions, which are always
identical to English.
Finally, this patch adds those consistently to all
the messages. Some messages didn't mention them
for an unspecified reason, and now they are mentioned
everywhere.
Bug: T360909
Change-Id: I3283c91b6b1d5fe9b48b1477cd454d9def3a7ded
A custom API error code and data similar to those used when an edit is
blocked by a normal AbuseFilter filter were accidentally added when
the feature was introduced. They should not be there, as the blocked
domains feature is not a normal AbuseFilter filter.
Hopefully nobody is relying on the format of this API response yet.
This commit changes the action=edit response for this case from:
{
"error": {
"code": "abusefilter-disallowed",
"info": "The text you wanted to publish was blocked by our filter. The following domain is blocked from being added: example.edu",
"abusefilter": {
"id": "blockeddomain",
"description": "blockeddomain",
"actions": "disallow"
}
}
}
to:
{
"error": {
"code": "abusefilter-blocked-domains-attempted",
"info": "The text you wanted to publish was blocked by our filter. The following domain is blocked from being added: example.edu"
}
}
Change-Id: I61ccc8f44b63e5cd0f11b1fe9a00ff60104a6249
Instead of having separate methods for each variable,
have one method which can work not only with "_links",
but with any array of strings.
Change-Id: I05f1b1cbd15f283b314c72259f183f7788e4e214
It is a common pattern to avoid SELECT * and use the fields used by
the application to avoid loading to much data into memory and maybe use
performance benefits when fields are covered by index.
Change-Id: I08a399f1b6a66442317b151be5386c9d2485f1fb
Constructing a Status like this does not make sense (and I want
to deprecate it in I0675e557bb93a1c990fa923c50b9f6ee8a9836c8),
because the parameters are ignored by most Status methods:
$error = Message::newFromSpecifier( 'abusefilter-blocked-domains-attempted' );
$status = Status::newFatal( $error, 'blockeddomain', 'blockeddomain' );
But it worked here, because FilteredActionsHandler::getApiStatus()
used a deprecated method that allowed inspecting them.
I feel like this code in BlockedDomainFilter has been added to make
the tests pass without thinking about what it actually does, which is
to output a bunch of useless incorrect data in API errors.
I'm not sure if we can remove that now without breaking API
compatibility, so add the useless data in FilteredActionsHandler
instead, closer to where it's output.
Change-Id: Ic12241bd3029bc1b0e7a0023689a2be35ccd30a8
From Status class documentation:
> The recommended pattern for Status objects is to return a Status object
> unconditionally, i.e. both on success and on failure -- so that the
> developer of the calling code is reminded that the function can fail, and
> so that a lack of error-handling will be explicit.
Change-Id: Ie6a55e297a35374fbdef880dd40e65f5cd00b6bf
The static method has already been migrated.
Also rewrite the test cases to avoid non-static provider (T337144).
Change-Id: Ibf98539f442e1ba8a9e9eb510784d40778123f17
MediaWiki core change Icb8822def9ce56f42ff52a8e469bb08d61d576c6
improved the type hints for OutputPage::addWikiMsg(), resulting in
two new errors:
* AbuseFilterViewEdit.php: False positive, update suppression
to include new error code.
* SpecialAbuseLog.php: Genuine bug, the return value of
Status::getErrors() can't be used directly as a message key.
I have another change pending that introduces a nicer way
to do this: Ibc4ce11594cf36ce7b2495d2636ee080d3443b04,
but in the meantime, make do with the available getters.
Change-Id: Iee0e87496e27a5261adccb977361b3ccf4c9ee2c
InsertQueryBuilder does not ignore insert of no rows,
adding some conditions to avoid calling the query builder
Change-Id: I1752b90cc3a7ec3a7f9ee32a1873bf8c82b6e02e
Introduced in 2019 with 4c8dac4dc6. Redundant since 2020 with
commit c6c62e2c8f in MediaWiki core.
Bug: T139216
Change-Id: I51e9fc3899cf5505917d7899a395350dd86f5c0b
Make the order of the messages that describe
operators and functions in the en.json file
identical to their order in
KeywordManager::BUILDER_VALUES, which is also
their order in the actual UI of the filter editor.
This only reorders the mesages in the en.json file.
It's not supposed to change anything in
the end users' experience, but it will change
the order in which translators on translatewiki.net
see them.
This is a cleanup step towards removing
the explicit operators from the messages,
as suggested in T360909, and this reordering
is hopefully useful even without that change,
for general consistency.
Comments about particular messages:
* abusefilter-edit-builder-vars-timestamp-expanded
is moved to the very end because, despite its key,
it's not actually used in the filter builder.
* old-text, old-html, and minor-edit are moved towards
the end because they are outdated. They are listed
separately from BUILDER_VALUES and they are not used
in the filter builder UI, but they are used in the logs
of previous actions. This patch adds a code comment
for the benefit of developers who touch that code
in the future.
Bug: T360909
Change-Id: I86ecdca5a6173b9068d5e968e69c57c74a379888
And more db clean ups:
- Use QueryBuilders
- Stop relying on actor migration to simplify query building
- Using expression builder in one case.
- Change the default actor migration stage to read new and write both.
Bug: T354194
Depends-On: I7c116cab0c748707d9a9fd17feeffe26e7d188ec
Depends-On: I74002911749335f4323a03fb430d02f936771b7e
Change-Id: Id84d1db7a2991f3cccc2f4f1502ba77643ddef24
The following sniffs are failing and were disabled:
* MediaWiki.Commenting.FunctionComment.MissingDocumentationPublic
Change-Id: I6075c76d53a899aac56af027f9a956a6b9e6a667
Why:
* The AbuseLogger::insertLocalLogEntries calls the CheckUser
extension to insert the abuse filter log entries, so that
checkusers can see these log entries in the results.
* However, these logs can be inserted on GET requests (such as
when a filter matched a autocreation of an account). This means
that the TransactionProfiler warns about writes.
* Updating the writes to occur on POSTSEND and also silencing the
expectations about not performing write queries should prevent
the logstash spam being caused by the TransactionProfiler related
to this code.
What:
* Make the call to Hooks::updateCheckUserData occur on POSTSEND
instead of PRESEND.
* Silence the TransactionProfiler for 'EXPECTATION_REPLICAS_ONLY'
only for the call to Hooks::updateCheckUserData.
Bug: T359648
Change-Id: I08e1674ff4dca386c046374c77dd31b4b29bb41e
Why:
* An AbuseFilter variable is needed that allows filters to determine
what type the current user is. That is, whether the user is an
IP address, temporary account, named user or external user.
* Currently filters implement this by inspecting the value in
the 'user_name' variable, but this is likely to break when
temporary accounts are enabled as IPs would be hidden.
* Giving a dedicated variable that indicates the type of the user
allows filters to work out this information without having to
know the specific username of the user before performing the
check.
What:
* Add the 'user_type' variable which is lazily computed. It can have
the value 'named', 'temp', 'ip' or 'external' depending on the
type of the user. If the user does not match any of these, then
the value is 'unknown'.
* Replace call to deprecated User::newFromIdentity with a use of the
UserFactory service that is dependency injected.
* Add and update tests to ensure consistent test coverage.
Bug: T357615
Change-Id: Ifffa891879e7e49d2430a0330116b34c5a03049d
This patch solves a pending TODO which is to remove the ::factory()
method from the AutoPromoteGroupsHandler class. If the cache instance
is injected, we'll use it otherwise we'll default to a HashBagOStuff.
Bug: T358346
Change-Id: I2bc414da8733431d1d11025e954282fc7c73aa80
Why:
* AbuseFilter can send AbuseFilter logs to CheckUser if they are
not being sent to Special:RecentChanges.
* However, if this action is indirectly causing the creation of
an account (such as through temporary account auto-creation),
the log entry is sent to CheckUser before the temporary account
actually exists in the 'user' table.
* This causes a CannotCreateActorException, as the performer does
not exist on the wiki just yet and therefore cannot have an
actor ID until the temporary account is created.
* This exception can happen if the AbuseFilter filter only creates
a log entry and does not prevent the edit, so would not be
necessarily fixed by T334623.
* Sending the logs to CheckUser on PRESEND avoids this, as the
user will exist by the time that PRESEND is run but still allows
any failures to cause an exception which can be seen by the user.
What:
* Wrap the call to Hooks::updateCheckUserData in AbuseLogger
::insertLocalLogEntries in a DeferredUpdate which is set to run
on PRESEND.
Bug: T358632
Change-Id: Ia615fce3e26b88d5457ecc01231044b326b79973
This feature never worked very well, and the original wish
https://w.wiki/7ZsE didn't ask for a 2010 editor solution, anyway.
Rather than have AbuseFilterBlockedExternalDomainsNotification linger in
an unstable state, we remove the code entirely.
Bug: T347435
Follow-Up: I7eae55f12da9ee58be5786bfc153e549b09598e7
Change-Id: I88e87c4e0a2968b892394461b1227f4d15938e8e
Why:
* When CheckUser asks the AbuseFilter extension for modifications
to rows inserted into the CheckUser tables, the AbuseFilter
extension attempts to get the Filter user via User::newSystemUser
* User::newSystemUser can deadlock if multiple requests to create
the system user are being made at once.
* The CheckUserHander does not need to create the abuse filter system
and instead only needs to know if a given $user is the equal to
the FilterUser.
* As such the FilterUser service needs to provide a way to check if
a given $user is equal without creating the FilterUser.
What:
* Add FilterUser::isUserSameAs which returns a boolean value
indicating whether the Abuse Filter system user is the equal
to a given UserIdentity in the same way that UserIdentity::equals
is implemented.
* Refactor ::getUser to get the username for the filter user in
a separate method, so that the ::isUserSameAs method can also
use this method. Name this new method ::getFilterUserName.
* Add a test for the FilterUser service to ensure consistent test
coverage
* Convert the @covers and @coversDefaultClass annotations to be
a @covers for the class. This is because PHPUnit recommends this in
https://docs.phpunit.de/en/9.6/annotations.html#appendixes-annotations-covers-tables-annotations
Bug: T356275
Bug: T346967
Change-Id: I8a101781bb47612deabb0f2a06a398ac13e860e6
We are getting rid of the schema of implementing this interface and
calling self::READ_* constants, it's confusing, inconsistent, prone to
clashes and isn't really useful for non-ORM systems (which we are not)
Bug: T354194
Change-Id: I5d7a2c91a49311a6bdf6e56053c08610d4d6d110
Why:
* The AbuseFilter Special:AbuseLog/hide page has a form that allows
those with sufficent rights to hide abuse filter log entries.
* This form defines a custom action, which uses a URL including the
wgServer by calling ::getFullUrl.
* When on WMF wikis and using mobile view, the domain name includes
'm' and as such the wgServer is not the correct URL for the form
action in this case.
* HTMLForm by default uses ::getLocalUrl for the action and as such
Special:AbuseLog/hide should also use ::getLocalUrl to prevent
these issues.
What:
* Change the call to ::getFullUrl in HideAbuseLog::show for the
action text for the HTMLForm instance to instead be a call to
::getLocalUrl.
Bug: T355012
Change-Id: I6c909d5e6724dd620cf656c9a55439ed5d5c2fb4
This field gets added automatically when using the special page form but
is only shown to admins and other people who have access. It's not private
information (users can find it in history) but this is to avoid making
these admins an easy target for harassment (Talking to PM of moderation
team he agreed this is a good compromise).
Bug: T341626
Change-Id: I8410f39db54b96981b05de8e064fed65df30ef2f
