Commit graph

39 commits

Author SHA1 Message Date
Daimona Eaytoy 661a77f0eb Rename addStaticVars and related hook
This code was introduced with Iba59fe8d190dd338ecc8cfd682205bce33c9738b
and is unused since then. The name should highlight that those variables are not
supposed to be "static", i.e. immutable. Examples are: timestamp, spam
blacklist, site name, site language. These are not immutable, but rather
"generic", and they're known even without an ongoing action.

Also add an RC row param and update docs.

Change-Id: I402f04585e9154059fc413e527e39dcb8e6b3d7c
2020-02-09 14:29:08 +01:00
Daimona Eaytoy 3f83e57ad7 Factor out variables-related methods
This is another step needed to reduce the size of the gigantic
AbuseFilter and AbuseFilterHooks classes. It also makes many methods
non-static, for more testability.

Note, this layout is still not final. We should somehow merge the
functionality of VariableGenerator and AFComputedVariable, for which
I already have plans.

Change-Id: I366d598b69ad866496b7cb0059e0835c02e54041
2020-02-07 20:27:26 +00:00
libraryupgrader a14ec744f7 build: Updating composer dependencies
* mediawiki/minus-x: 0.3.2 → 1.0.0
* mediawiki/mediawiki-phan-config: 0.9.0 → 0.9.1

Change-Id: I119f4d56cce674302f34e938e598e6cc6bf28dc0
2020-01-28 17:51:38 +00:00
Ammar Abdulhamid 641aeebbcf Replace deprecated IP class with IPUtils
Bug: T242556
Change-Id: If8e9034885726b673d1500fa8b538b5302e66165
2020-01-24 18:27:26 +01:00
jenkins-bot 5db1032618 Merge "Simplify throttling code" 2020-01-22 17:19:52 +00:00
jenkins-bot 81fd6af030 Merge "Actually record all filters in total_filters" 2020-01-22 17:19:50 +00:00
jenkins-bot 70d31da673 Merge "Stop using deprecated stuff with easy replacements" 2020-01-22 16:44:57 +00:00
Daimona Eaytoy 87459ec679 build: Upgrade phan
Depends-On: I6d538ce3ca7fd2d495c2bafbab7cc279da69db1c
Change-Id: Ic8c3a01a5c37fdf461f4fd5598e597eb9c9073d3
2020-01-19 18:48:51 +00:00
Daimona Eaytoy 10c2fe7151 Stop using deprecated stuff with easy replacements
This patch is mostly replacing Revision::* constants,
Wikimedia\(restore|suppress)Warnings, and wfWikiId.

Change-Id: I13544cc3e12955a9376ccce3c120e2cee1f2ee2e
2020-01-08 14:59:30 +01:00
Daimona Eaytoy c54e2fc180 Simplify throttling code
Change-Id: I54ebdf0bc61d5628d1755b75232a934358b112ff
2020-01-07 17:52:16 +01:00
Daimona Eaytoy 759cb38bf5 SECURITY: Unbreak blocks shorter than one hour
Bug: T238768
Change-Id: I8820a6e2953255f409ff8ddc2b41dcef2f338e37
2019-12-04 18:46:40 +01:00
Thalia 63eb7eafb7 Use AbstractBlock setters and getters instead of deprecated properties
Change-Id: I01728f919254a9435f051af3fc390eb80ca8d17e
2019-10-20 00:35:00 +01:00
Daimona Eaytoy b9e4475985 build: Upgrade mediawiki-phan-config to 0.8.0
This is to verify that our CI is able to handle the new version.

Bug: T235049
Change-Id: Ib7427e15f673a575738489476e604c387f449ddd
2019-10-09 19:12:51 +02:00
Daimona Eaytoy 703835e835 Drop HHVM support
Change-Id: Ib7ccb4f68278ba8ca009e9d18e9d8b127f799cde
2019-10-03 12:27:18 +00:00
Daimona Eaytoy 142ad5faf8 Actually record all filters in total_filters
Change-Id: If6d15423e0a96c31666690e4fe8c7aeb439f82e8
2019-09-29 11:02:29 +02:00
Daimona Eaytoy e2570a4c2b Actually provide a StatsdDataFactory to the parser
Follows-up Ib934be34a953166fe1b94cfe8ed216afe3b906ca

Bug: T156095
Change-Id: Ia8df84cf7c43071f304ce729b811dfd5aa96b951
2019-09-19 19:06:14 +02:00
Daimona Eaytoy 489da0d229 Add a 'strict' option to VariableHolder::getVar
This will help mitigating problems like T230256 by enforcing that the
requested variables must exist. For now, it will only log bad usages,
thus providing a way to identify affected filters and fix them.

Bug: T230256
Change-Id: I7a61916576e444a56f0e07da7b6e5033346226bd
2019-09-04 18:19:23 +00:00
Daimona Eaytoy d51ca862c6 Move parser tests to /unit
IMHO these can be considered unit tests; they were already fast, but now
they're executed in an instant.
This requires several changes: 1 - delay retrieving messages in
AFPUserVisibleException, to avoid having to deal with i18n whenever we
want to test exceptions; 2 - Use some DI for Parser and Tokenizer.
Equivset-dependend tests are also moved to a new class, thus helping to
fix the AF part of T189560.

Change-Id: If4585bf9bb696857005cf40a0d6985c36ac7e7a8
2019-08-28 16:36:37 +00:00
Daimona Eaytoy 8e166f10d6 Refactor and speed up non-parser tests
Some of these are transformed into real unit tests, while the
AbuseFilterSaveTest class is refactored to avoid using the DB and to use
a lot more of mocks and DI.

Depends-On: I22743557e162fd23b3b4e52951a649d8c21109c8
Change-Id: Id8412e2b8a4e873fd4821ecc1a3c95710be9a870
2019-08-27 16:24:27 +00:00
jenkins-bot 89524790d5 Merge "Add a hook to determine whether the current action should be filtered" 2019-08-25 18:45:07 +00:00
jenkins-bot d0b30c2534 Merge "Make parser aware of the filter it is parsing" 2019-08-20 12:50:26 +00:00
jenkins-bot 61bb3ff3e8 Merge "Various changes for blockautopromote" 2019-08-14 23:59:08 +00:00
Daimona Eaytoy 27578d7ba0 Various changes for blockautopromote
Problems fixed:
 - Don't hardcode duration in the message
 - Move duration to a constant
 - Fix wrong parameter order for AbuseFilter::blockAutopromote
 - Log a warning if we cannot block autopromotion
 - Remove the $performer parameter, as it should only and always be the
 filter user.

Bug: T230296
Change-Id: Ice9e4b21033c430cf1fd34182c63ca64ad2f5d3e
2019-08-14 18:50:43 +02:00
Daimona Eaytoy 1197eb6b41 Make parser aware of the filter it is parsing
This information will mostly be used for debugging purposes.

Change-Id: Ia1bcc2acc22aba97d855382b5b173ac3d5f2c54b
2019-08-13 15:22:38 +00:00
Daimona Eaytoy 7edf12dbc6 Add a hook to determine whether the current action should be filtered
Bug: T229252
Change-Id: I126013b6c516eebe7273fb85f2c5681844e757be
2019-08-13 10:31:29 +02:00
Daimona Eaytoy ff40204ef1 Gracefully handle blockautopromote failures
Instead of returning a successful message, return null and log a
warning. Also, make autopromoteBlockKey public + internal and use it
from Hooks instead of duplicating the logic.
Follow-up: I03feb05218789a3b73a31c9a94216daafcb7c145

Change-Id: I8ce96d1bd0239003f8ee6a45f412b9502d542a18
2019-08-10 15:18:30 +02:00
Aaron Schulz 9e44f1a9e9 Move "block-autopromote" key from $wgMainStash to 'db-replicated'
Keep the key mutation methods in the AbuseFilter class

Bug: T227376
Change-Id: I03feb05218789a3b73a31c9a94216daafcb7c145
2019-08-07 01:09:13 +00:00
Daimona Eaytoy 2bdb44d58b Overhaul Blockautopromote action
As for all mostly unused consequences, blockautopromote has a couple of
major problems: first, it blocked the status for a random time between 3
and 7 days, which to me makes no sense at all (is it some sort of
casino?), and this patch fixes it to 5 days. Second, nothing was logged,
not the blocking nor the unblocking. Here I'm adding a LogHandler for
two new sub-actions of 'rights' to keep track of both action.

Bug: T49412
Change-Id: If48a48f5b8baaf9e77c0826466f5d03bb7f691d0
2019-08-05 22:27:49 -04:00
Daimona Eaytoy 6ef2cf523b Profiling: don't count time for operations shared with the edit
Parsing wikitext and retrieving its links are operations which we share
with the edit, so that if a filter does that, it won't be done later
upon saving.
Thus, add a static variable to subtract such time and avoid to erroneously log as slow any filter using those variables.

Bug: T219092
Depends-On: I24fbd41ac188a9cf6a7d3ca33dce349aedc9faa6
Change-Id: I7c0170167b508132cd16e566c654a6c98dd683e9
2019-08-04 20:12:10 +00:00
jenkins-bot 4347331692 Merge "Reset all filter profiling data at once" 2019-08-04 18:53:57 +00:00
jenkins-bot 19182606c1 Merge "Merge global profiling keys" 2019-08-04 18:40:14 +00:00
Daimona Eaytoy 2742f24bca Reset all filter profiling data at once
Instead of scattering the process all over the code (and doing it
together with checking if the key already exists).
Wrap the logic in new methods for better readability.

Depends-On: Ib12e072a245fcad93c6c6bd452041f3441f68bb7
Change-Id: I24fbd41ac188a9cf6a7d3ca33dce349aedc9faa6
2019-08-04 18:03:14 +00:00
rarohde d022377578 Merge global profiling keys
The last step of the profiling overhaul. See T53294 for the original description by Dragons flight.

Note: Here I'm adding a FixMe for a problem which already exists in the code
and the child patch will fix it.

Bug: T53294
Depends-On: I2d8c8f8278073a9420e3eb373fb89a655925618a
Change-Id: Ib12e072a245fcad93c6c6bd452041f3441f68bb7
2019-08-04 17:59:58 +00:00
Daimona Eaytoy 4f8811bc3b Update cache key version for data in stashed edits
In I1dc3be6da1cc9e03bc47e8f8c867089ad0100f6f we added fields to the array.
Update the version to avoid PHP errors while upgrading the wikis, for edits
stashed before the upgrade, and saved afterwards.

Change-Id: I5489b556b1b0e9cb2af862dbfa0621909a5e355d
2019-08-04 16:36:23 +00:00
Daimona Eaytoy c3db63714e Use milliseconds for time profiling
Instead of seconds, and round the average condition at 1dp instead of 0.
Split from child patch by Dragons flight.

Depends-On: I2d8c8f8278073a9420e3eb373fb89a655925618a
Change-Id: I339aed5f8c1d49714e7927ce49286f9ce6c839f5
2019-08-03 23:24:46 +00:00
Daimona Eaytoy 0b7902fe6e Move per-filter matches profiling to per-filter data
They're currently stored separately, so move matches count together with
other per-filter data to keep it consistent. This also removes a
parameter from filterMatchesKey, as it's not needed anymore.
Split from child patch by Dragons flight.

Bug: T53294
Depends-On: I8f47beb73cfc1b63c4b3c809fc6d65a1e66ee334
Change-Id: I2d8c8f8278073a9420e3eb373fb89a655925618a
2019-08-03 23:22:20 +00:00
Daimona Eaytoy d04a0d3afc Store per-filter profiling in a single key
Instead of having three keys, one for total actions, one for time and
one for conditions. This has several benefits: first, it avoids race
conditions which could happen having different keys. Second, it's much
more performant. Third, the code is also clearer to understand,
and more uniform with the one for global stats.
Split from child patch by Dragons flight.

Bug: T53294
Depends-On: I1dc3be6da1cc9e03bc47e8f8c867089ad0100f6f
Change-Id: I8f47beb73cfc1b63c4b3c809fc6d65a1e66ee334
2019-08-03 11:39:27 +00:00
Daimona Eaytoy 4acb266e90 Save profiling data and vars in cache when running filters
This is the proper solution to replace
Ia8e38ba25d1989fe71714d2b76891c4587921466, using a class member and an
additional method. Plus, change checkFilter not to accept a prefix, but a boolean indicating if the filter is global (as that's how it's used currently).

This change also fixes an issue which caused profiling data for local
filters to be mixed with profiling data for global filters with the same
ID.

Depends-On: Iafc142d2e5ba7aa0fb0d3265fa05cace27679738
Change-Id: I1dc3be6da1cc9e03bc47e8f8c867089ad0100f6f
2019-08-02 22:54:30 +00:00
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