Commit graph

58 commits

Author SHA1 Message Date
Daimona Eaytoy 4720c97530 Add a new class for methods related to running filters
Currently we strongly abuse (pardon the pun) the AbuseFilter class: its
purpose should be to hold static functions intended as generic utility
functions (e.g. to format messages, determine whether a filter is global
etc.), but we actually use it for all methods related to running filters.
This patch creates a new class, AbuseFilterRunner, containing all such
methods, which have been made non-static. This leads to several
improvements (also for related methods and the parser), and opens the
way to further improve the code.
Aside from making the code prettier, less global and easier to test,
this patch could also produce a performance improvement, although I
don't have tools to measure that.
Also note that many public methods have been removed, and almost any of
them has been made protected; a couple of them (the ones used from outside)
are left for back-compat, and will be removed in the future.

Change-Id: I2eab2e50356eeb5224446ee2d0df9c787ae95b80
2019-07-23 19:06:27 +00:00
Daimona Eaytoy 3258eeed69 Add normalizeThrottleParameters script to update.php
Bug: T203587
Bug: T203336
Bug: T203584
Bug: T203585
Depends-On: I7831dbb0bab55807392ac1f7915d6cb0cb713593
Change-Id: Ideaae7b58e0ffa606095aac4a9e5d21c6bdf11d2
2019-07-17 12:36:08 +00:00
Daimona Eaytoy c6a9f3517a Really drop afl_log_id from update.php
Follow-up of 0b925da36e, somehow I forgot
to add the removal code for MySQL and SQLite to the Hooks.

Bug: T214592
Change-Id: If0d1d5430573273784ff6f6e338b0c2199f6d7bb
2019-07-09 16:51:28 +00:00
Aaron Schulz 2cf7b58434 Convert wfGetDB() calls to using getConnectionRef()
This handles the logic of calling reuseConnection() automatically

Change-Id: I9328e709fe5d81099338a31deef24d34db22d784
2019-07-04 15:09:32 -07:00
Daimona Eaytoy 0b925da36e Drop afl_log_id
Field unused since its introduction.

Bug: T214592
Change-Id: I1f4f775e9678de5184672251631a490e4eb81764
2019-06-28 17:55:55 +00:00
Daimona Eaytoy a8e8611509 Remove log_ids meta-variables
This is the second part of removing meta-variables. To achieve this, a
static property is added and another one removed.

Depends-On: I7f60df24dc8e706af289ebbbde7536c0baf8d5c3
Change-Id: I5b29ff556eca45fe59d15e2e3df4d06f1f6b3934
2019-06-19 14:44:56 +00:00
Daimona Eaytoy c3ee5e7251 Simplify static properties in AbuseFilterHooks
Avoid all those data types (i.e. use null instead of false), use camelcase, make them private. Also, remove some logic to handle $lastEditPage being Article, as it can only be WikiPage.

Depends-On: I5a9db6e7c4356c9662a0b0a51e66252555b3d998
Depends-On: I359a618ffc4e45ce1fb70f2d1aa99a6668609e36
Change-Id: I7f60df24dc8e706af289ebbbde7536c0baf8d5c3
2019-05-25 16:27:21 +00:00
Daimona Eaytoy 864d2b7e03 Don't run filters with null title
As all title variables would be null, and the result pretty meaningless.

NOTE: Please vote V+2 and submit manually. I359a618ffc4e45ce1fb70f2d
should then be +2ed right after that. This way, there is no need to create
two more patches just for a handful of tests being broken for a minute.

Bug: T144265
Bug: T219030
Depends-On: If6b91711534c0d60e1aa27bd5748c3023e29f376
Change-Id: I5a9db6e7c4356c9662a0b0a51e66252555b3d998
2019-05-25 16:27:08 +00:00
Daimona Eaytoy f56562f583 Add tests for global filters
Another crucial part to have covered. Also clarify that
AbuseFilterCentralDB can be of the form "dbname-prefix".

Remove a filter used for profiling and replace it with a global one:
we're still fine, and the list is kept shorter.

Bug: T201193
Depends-On: I5ee7ba44a6cd82a5ddb24fb4127af04d96e647f4
Change-Id: If6b91711534c0d60e1aa27bd5748c3023e29f376
2019-05-24 16:58:23 +02:00
Thiemo Kreuz 1c5accd90a Remove problematic array type hint from hook handler
It's possible this parameter is null, as demonstrated in Id2caa44.

Change-Id: I69bf0d70552fb049aa1c93bb12bcb5cc9e457c53
2019-05-18 08:50:22 +02:00
Thiemo Kreuz 3dece9ef8c Make use of PHP's list() feature where possible
Change-Id: If2b6c95a319800dd4944ecc0d7a8d3a819c846c1
2019-05-15 16:12:51 +02:00
jenkins-bot 06cac3682e Merge "Replace deprecated cache-related functions" 2019-05-01 16:27:56 +00:00
Daimona Eaytoy 72c2be7a18 Remove $wgAbuseFilterRuntimeProfiling
The reasoning is similar to the one of the parent patch (Ia5c477edc8733bb1994cb6d01e1371ed496c8bcb). Plus, it records runtime metrics on action different than edits, as there's no reason not to do it.
No performance issues in production.

Bug: T191039
Depends-On: Ia5c477edc8733bb1994cb6d01e1371ed496c8bcb
Change-Id: Ib1112e2fefd0631550d386ba87e5f87db84c3036
2019-03-23 11:31:18 +00:00
Daimona Eaytoy 89520e2353 Remove $wgAbuseFilterProfiling
This variable was introduced to selectively enable profiling because
stats recording was bad for performance. Nowadays, stats are recorded in
a deferredupdate and don't harm performance anymore. Thus, this variable
can be removed and profiling be enabled by default.

Bug: T191039
Depends-On: Ib5fdeb75c1324f672b4ded39681f006fde34b4d1
Change-Id: Ia5c477edc8733bb1994cb6d01e1371ed496c8bcb
2019-03-23 11:31:11 +00:00
Daimona Eaytoy f2c1beec44 Replace double-equals with triple-equals
Since double-equals are evil. I left some of them in place where I
wasn't sure, but I may be changed some which were intended to be
doubles. It could be a good idea to delay merging this patch until we'll
have more code coverage.

Change-Id: I1721a3ba532d481e3ecf35f51099c1438b6b73b2
2019-03-22 16:12:13 +01:00
Daimona Eaytoy bae9c5bb8f Use lowercase for built-in variables
The uppercase is just a leftover from a long time ago. Currently,
variables are case-insensitive, and we already perform a strtolower when
saving them. Since most parts of the code already use lowercase, the
uppercase leftovers only make it harder to grep the code to find
variables. As a bonus, make Ace recognize variables in a
case-insensitive fashion.

Change-Id: I72933fcc9952fc1aabf6464b2fc0b04ec39c024b
2019-03-17 14:23:11 +01:00
jenkins-bot dc0c5cc5ac Merge "Make uploads testable" 2019-03-17 11:06:43 +00:00
Daimona Eaytoy 3211c71739 Make uploads testable
This patch adds missing methods for testing a filter against uploads in
RC. Please note that (as discussed below) using wfFindFile could be
relatively expensive (as it will be executed for 0-100 RC rows).
If this is true, then we should either use another method (but I
couldn't find a suitable one), or simply reduce the amount of testable
uploads to a reasonable limit.

Bug: T170249
Change-Id: Id406d4e1571873f49bb11e69029311b24ececf49
2019-03-17 11:32:56 +01:00
jenkins-bot de4e971c72 Merge "Remove usage of MakeGlobalVariablesScript hook" 2019-03-17 10:32:30 +00:00
Matěj Suchánek 99c2749511 Fix issues with SQLite
Bug: T199507
Change-Id: I273de5ebafeee76458a5b873e893044b683c34ca
2019-03-01 10:44:58 +01:00
Daimona Eaytoy 90df3560b1 Replace deprecated cache-related functions
Some ObjectCache:: methods are soft deprecated since 1.28. Remove them
now, since the replacement is easy.

Change-Id: I713781d5e98238a1c194e97b5faae488a8ac190d
2019-02-09 16:01:57 +00:00
Daimona Eaytoy a207cf22f7 Unbreak tagging for createaccount actions
Tagging doesn't work for account creations, and probably never did. This
is because we used a wrong identifier for such actions. This patch fixes
the problem, although in the long term we should find a smarter way to
apply tags.
Also, clean AbuseFilter::$tagsToSet if the action will be prevented.

Depends-On: Ia8e38ba25d1989fe71714d2b76891c4587921466
Change-Id: I8edcca17ecdcf71397cc9b0d101e8b13ac112047
2019-01-23 21:25:47 +00:00
Daimona Eaytoy fca80fa976 Remove usage of MakeGlobalVariablesScript hook
This is an old leftover, used to add global JS variables in a convoluted
way: using a hook and a total of 3 static properties. We can safely
remove all of this and just call OutputPage::addJsConfigVars, which BTW
is already called where we need it.

Change-Id: Ifad0618fa93b0c7a7e8b23f596234e622aa8846a
2019-01-21 14:27:57 +01:00
Daimona Eaytoy 6217ffb928 Remove unused stuff
Variables declared but never used, redundant code, and old leftovers.

Change-Id: Ic51044a45a1b49ad6c7af06c646b11893411a7cd
2019-01-18 17:04:19 +01:00
Daimona Eaytoy 93e8cb5ac5 Tune logging channel
As follow-up of I10b1fd2d9bdfe518089c053d77fef568170ecb65, use
'AbuseFilter' instead of 'AbuseFilterDeprecatedVars' as channel name.
Raise level for null-title filtering. Since with a null title
several things are likely to break, a warning is more appropriate here.
Tweaked the message as well, to include the bug number and to avoid
pointlessly including the title (which is null).
Lower the level for stashedit hit/miss (as it's really spammy and not
that useful right now).
Use 'abusefilter' instead of 'AbuseFilter' for statsd so that everything
has the same prefix.
Also raise the level for parser exceptions and unrecognized
consequences.

Change-Id: I1f9988155e924232b201281795cd322636da8082
2019-01-16 08:56:22 +00:00
jenkins-bot b1e8f38c64 Merge "Replace RecentChange::$mAttribs with getter functions" 2019-01-11 20:01:12 +00:00
addshore a6a93be530 Pass MCR AF text into newVariableHolderForEdit
Follow up to Idbb3a70d08a195dfa21422e07f593d1eeba4521d

This also fixes the fetching of text for the stash edit code path
which was missed by the previous patch.

This now also uses the full old text in the variable holder.

Bug: T213453
Change-Id: Ib80bc6385ebb5dd82bb1a384dd0e162608bfcbfa
2019-01-10 23:42:58 +00:00
addshore 3e93c06223 Use slot in onEditFilterMergedContent
Related to If3c4592eb6dade6960463abfda017af35d04f563
in Wikibase, needed for SDoC.

Bug: T213453
Change-Id: Idbb3a70d08a195dfa21422e07f593d1eeba4521d
2019-01-10 20:57:30 +00:00
Daimona Eaytoy fda8f01431 Replace RecentChange::$mAttribs with getter functions
The RecentChange class has several getters and setters for the $mAttribs
property. Although the property is public, it's saner to use such
methods.

Change-Id: Ie8e37e80fdcf2917ee0e87b2a409f0afb91a4f92
2019-01-02 11:36:57 +01:00
jenkins-bot e6ca0f288d Merge "Really disable the minor_edit variable" 2018-12-31 02:21:56 +00:00
Daimona Eaytoy 2fc56ce014 Use array_unique on the array of tags to add
Otherwise calling bufferTagsToSetByAction multiple times makes the list
grow, and IIRC the core doesn't call array_unique on the tags to apply.
Also clean the list after applying tags.

Change-Id: Iebbdeac7898b35beea79aa3d0cdf9d0fb265d726
2018-12-29 15:19:02 +01:00
Daimona Eaytoy 921db0397e Really disable the minor_edit variable
The variable was disabled with I7f13773766e12f3d4b86451fdf3ae23e067ac373
in 2016, but not in the same way as old_text and old_html were disabled
in 2009. This patch uses the methods introduced with
Ife168522e6b1d8eb94ebbb8a16ae8831ec1dc497 to disable minor_edit in a
standard way, so that it won't be showed in new AbuseLog entries, and
won't be usable when writing filter syntax.
A warning will be emitted if a pre-existing filter is using it, so that
we'll be able to completely disable it in the future.

Change-Id: I5ad5219ee19a5e6ba2bfdffb4e0aad63c8951491
2018-12-29 14:14:27 +01:00
Daimona Eaytoy f49d4e5caa Emit debug logs when filtering without title
We have two situations where we try to execute filters without a title.
However, the code doesn't handle it correctly: some points expect $title
to actually be a Title object, and we also pass it around using a hook
which explicitly says it always pass a Title. This patch adds two debug
points to help understand why we end up with null titles, so that we can
fix it upstream.

Bug: T144265
Change-Id: I35bfc483a0c69a5cbd38eae8ba299189955fa1ec
2018-12-13 20:34:21 +00:00
Daimona Eaytoy 206bdc1f6a Use the updated TitleMove hook to filter move actions
For several reasons:
*We're not really checking permissions (and the hook previously used is
meant to be used in such case)
*We'll show a cleaner error message (i.e. without the "You do not have
permission..." part)
*Filtering will happen closer to the actual move

Bug: T208907
Depends-On: I4733724075b7514e9db59e7be772d9409aa9da87
Change-Id: If88f736a446247f8b4b13c055c641d56f544d1ea
2018-12-04 18:58:04 +01:00
Daimona Eaytoy 7ca0941d1f Remove code for old global variables
Those two global config variables were removed more than 2 years ago, in
I790d39c2849922d7daf7479f298cd90cf30af129. Nothing else in the code
references them, so we can just remove the warning.

Change-Id: I427d06a80131447ea64064f45e84349f93e72cca
2018-12-02 16:24:09 +01:00
Daimona Eaytoy badde6ba75 Revert "Revert "Add typehinting for every object-only parameter""
This reverts commit 1ed75b4ae0.
Fixed the one which caused errors, by making articleFromTitle
only use WikiPage, instead of silently mixing WikiPage and Article.

Note for reviewers: this patch is identical to the one which was
previously +2ed, which was mostly correct. To see the actual change,
diff AFComputedVariable with 1..current.

Change-Id: I6747eaed861af6c40a3b1610aebcc1174296e9ed
2018-11-15 10:09:16 +01:00
Jforrester 1ed75b4ae0 Revert "Add typehinting for every object-only parameter"
This reverts commit 69d7669069.

Reason for revert: Causing UBN train blocker

Bug: T207220
Change-Id: I3445d9b3065149e2beb149e10fbbf5502b480f57
2018-10-17 01:22:23 +00:00
Daimona Eaytoy 69d7669069 Add typehinting for every object-only parameter
This patch covers every object-only parameter, adding a typehint for it
to avoid errors.

Change-Id: Iebf700621b9dbff78c3bd8f3c136ed15ef4b8d4b
2018-10-15 09:56:09 +02:00
Matěj Suchánek a3cc3dff75 Remove some $wgUser usage
Bug: T159299
Change-Id: I1613e2bb0c551cbadc0c57351fc40bd9e21abf52
Depends-On: I35adef06dfc799cddeddfa6c5eed53b8b1bb7282
Depends-On: Id19a6d883ac6e0cc9c26c923486bca0e414ecaa7
2018-10-14 11:24:52 +02:00
Daimona Eaytoy e60dacbbea Fix code comments
Fixed some comments adding explanations, fixing syntax, and parameter types
for docblocks. Also fixed some whitespace mess, and added a missing use
statement.

Change-Id: I3547c90bdaa2cab5443e8bf0c63b217fe6ba663f
2018-10-03 16:45:03 +02:00
jenkins-bot 3efc69960c Merge "Fix database schema for PostgreSQL" 2018-10-01 15:43:29 +00:00
Umherirrender 45e6fa932d Fix caller name in AbuseFilterHooks::fetchAllTags
Seeing {closure} in the logs as caller is not helpful

Change-Id: Id3bf5c7fd810d48dc04a167692b336b3ccba2eb4
2018-09-30 14:08:06 +02:00
Matěj Suchánek db50bef21e Fix database schema for PostgreSQL
Bug: T62639
Change-Id: I5ddb781a2971677410f4cb96e5fc5964e53c862a
2018-09-29 12:12:52 +02:00
Daimona Eaytoy 8094a49dcf Generate upload variables using new prefixes
This wasn't changed in I5c370b54e6516889624088e27928ad3a1f48a821 but
really needs to be merged, to avoid setting wrong variables. At the
moment this is still fine due to temporary overrides in
AbuseFilter::generateTitleVars, but this should be merged ASAP anyway.

Bug: T173889
Change-Id: I2e6058a6fa122470a30cd4a96c68eccc66e18ae4
2018-08-25 19:06:35 +02:00
jenkins-bot a762c82fe7 Merge "Add aliases for "_text" and "article_" variables" 2018-08-22 12:44:20 +00:00
Matěj Suchánek 10ad58a6f3 Migrate AbuseFilter suppress log
Also make entries in Special:Log/suppress filterable.

Change-Id: Ic23e724997e4748c8d0da8138aa73d31b17b7064
2018-08-21 16:05:54 +00:00
Daimona Eaytoy 6bc630cfef Add aliases for "_text" and "article_" variables
Variables regarding title (full list in task description) are quite
deceiving, since they use "text" instead of "title". As proposed in the
task, this is the first patch to add aliases for those variables and
slightly deprecate the old ones. In the future we may be able to replace
every occurrence (either with a search function or directly on the
database), but even a coexistence would be enough to avoid
confusion. A wfDebug log is generated whenever a deprecated variable is
parsed. The "article_" prefix is also changed to "title_", in the same
way as above.
Also, added a hook which other extension may use to specify their
deprecated variables, which will be handled the same as core ones.

Bug: T173889
Change-Id: I5c370b54e6516889624088e27928ad3a1f48a821
2018-08-21 16:59:56 +02:00
tinajohnson.1234 c9003fe1fa Use HistoryPageToolLinks hook to add a log link to history pages
Add an AbuseFilter log link to the subtitle of history pages.

Bug: T28934
Co-authored-by: Matěj Suchánek
Depends-On: I2e0e9e92d3fc303135b0eb9acf06b5fd120178a5
Depends-On: I58a3039b3755648bb0c8aaf87db48ace96ce9344
Change-Id: Ib89c48f2b8f3121ead184844844acee436e2fdd6
2018-07-27 11:25:12 +00:00
Daimona Eaytoy f016c6c95f Fix minor issues around
This fixes the following minor issues:
* In HistoryPager's getQueryInfo, afh_id was listed twice
* In AbuseFilter::translateFromHistory a field named "af_" was produced
if no actions were in use
* The topnav link "Recent filter changes" wasn't STRONGed on pages like
"Special:AbuseFilter/history/123"
* In checkAllFilters and AbuseFilter::getFilter, select from DB only the
fields that will be used.
* Simplify some inline comments and remove superfluous ones

Change-Id: If72b18bedac5e580487406e696aea1fd172ae45b
2018-07-07 12:11:39 +00:00
jenkins-bot 8fa73341cf Merge "Remove all not needed & from hook handler signatures" 2018-06-09 09:22:29 +00:00