Commit graph

644 commits

Author SHA1 Message Date
jenkins-bot c3dcd95733 Merge "Start making APFData members private" 2019-07-09 09:23:17 +00:00
jenkins-bot 9650f11d16 Merge "Fix error display on ViewEdit" 2019-07-09 09:02:12 +00:00
jenkins-bot 35ab35978b Merge "Add a new data type for non-initialized stuff" 2019-07-09 08:58:48 +00:00
Daimona Eaytoy 3aaeb20063 Start making APFData members private
$data and $type are meant to be read-only and should have getter
functions, but as usual they're just public. Add getter methods, a
comment with a @private annotation and remove usages in our codebase.

Change-Id: I5e51efc9f982a4e340b48d20cb1b38a75bb10021
2019-07-09 10:57:00 +02:00
jenkins-bot 6f0905541a Merge "Make AbuseFilterVariableHolder::mVars private" 2019-07-09 08:42:16 +00:00
jenkins-bot 69bebbb4ff Merge "Simplify action arrays" 2019-07-08 23:07:26 +00:00
Daimona Eaytoy 304b58d46a Make AbuseFilterVariableHolder::mVars private
This property is meant to be private, since it has all kinds of
getters/setters, aside from one which is introduced in this patch.

Change-Id: I217b1e22cabd3c0468c84b1d6a69a6ed3c6fa8e6
2019-07-08 16:25:10 +02:00
Daimona Eaytoy 5eff8f73b0 Fix error display on ViewEdit
This changes the buildFilterEditor function to be protected and to
behave consistently: so, instead of adding stuff to OutputPage inside it
and also returning other stuff to be added by the caller, the function
now adds everything itself.
Also, the message "you're editing an old version of the filter" is now
shown only if the user can see the filter.

Change-Id: I1f40af41c5de0f63aa6210a261928892da0b3f69
2019-07-08 16:11:33 +02:00
Daimona Eaytoy bc79962803 Add a new data type for non-initialized stuff
Split from I5a14d4b2bc3ffd9caaaa095f16f36b9b6009db05.
This adds a new data type to use for empty AFPDatas. Using NULL for that
makes it impossible to distinguish cases where we really got a null
value, and cases where there was nothing to parse.
For now, DNONE is the same as DNULL, but I've explicited DNULL where
necessary. A subsequent patch will make proper use of DNONE.

Bug: T156096
Change-Id: I69bfec45c76509fb1112641393f78e8d8834adcd
2019-07-08 15:35:02 +02:00
Daimona Eaytoy d8d4750e6a Simplify action arrays
The current form is awkward. They're all like
[ actionname => [ 'action' => actionname, 'parameters' => params ] ]
This is greatly confusing since adds a nesting level, and just
duplicates the actionname information (also, we actually never retrieve
it from the internal array). Instead, change all of them to be
[ actionname => params ]
which is a lot shorter and clearer (and easier to handle).
A similar case is handled in I8134ecc41fbecdbed99faf406e9e3ca91b6123b9
(see PS 8..10).

Change-Id: I34c040dbeb3ab01158fb3db22496def6ccaf72d9
2019-07-05 10:00:48 +02:00
Daimona Eaytoy b2af2f0bf5 Fix global caching
Use a more explicit TTL_WEEK, and add the version to avoid breaking the
world if we change the format.

Bug: T227299
Change-Id: I22705496ed8541c3dd9b643d78dff8886f4ff070
2019-07-05 09:17:57 +02: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
jenkins-bot 0d001e3d6f Merge "Disallow consecutive comparisons" 2019-07-04 20:43:57 +00:00
Daimona Eaytoy 85b46268f2 Rename the filterAction hook and add a parameter
The 'AbuseFilter-filterAction' hook is deprecated in favour of a new
'AbuseFilterAlterVariables' hook, which provides a User object and has a better name, since it reflects what it should be used for, and doesn't include the name of a function which will be removed. The hook will be hard deprecated in a subsequent patch, to avoid test failures.

Depends-On: Ic0ecc8746e2883c746bef815a0fee4131f1a0646
Change-Id: I212b1e09e9c05d487d96b2f4c28f2a613e6ff3cf
2019-07-04 18:10:47 +00:00
Daimona Eaytoy b7f1a7d459 Enforce saving of full abuse_filter row in cache
This is somehow a follow-up of
Ieb04f019453033c275e211cfc9fd68d5d7c392ef. A new method is introduced to
cache a filter, which checks that all fields are there.

Depends-On: I7c1ea17adf7f42cf9260d416906bfbf3b8a20688
Change-Id: Ic0ecc8746e2883c746bef815a0fee4131f1a0646
2019-07-04 18:10:33 +00:00
Daimona Eaytoy 7398730563 Disallow consecutive comparisons
As explained on phabricator, they don't work with shortcircuit, so they
already fail for all filters using them. Plus IMHO it's an unnecessary
deviation from PHP's behaviour, given that this syntax doesn't do what
users may expect.

Bug: T218906
Change-Id: If9e7545e14044c8dc3b4163bb6fca8ab0683b9fa
2019-07-04 19:15:07 +02:00
DannyS712 eb278479d5 Add help links to special pages
Bug: T226938
Change-Id: I50a76733b3b8d4ee72ccc6816b58a67a66b2f603
2019-07-03 16:06:16 +00:00
Daimona Eaytoy 6ea767f171 Tweak methods related to global filters
To make the switch to afl_filter_id and afl_global easier.

Bug: T227095
Depends-On: Ie550889495232b534c0f9aec31039cf21b2135b1
Change-Id: If557bad8f5c1a6d15e3556e4bfbd0330d7d49c59
2019-07-02 17:02:50 +02: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 6100955242 Use more verbose names for filter IDs
Follow-up of Ie550889495232b534c0f9aec31039cf21b2135b1, suggested by
Krinkle.

Change-Id: Ia8f40644c7f4a6ed53186a5edab5df1bd313166a
2019-06-25 18:20:32 +00:00
Daimona Eaytoy 382751a707 Move conditions-related stuff inside AbuseFilterParser
Instead of relying on static methods and members in the AbuseFilter
class, move everything related to conditions inside the Parser, as the
amount of used conditions is something pertaining a single
AbuseFilter(Caching)Parser instance.
This change requires changing some signatures and adding parameters,
but will make introducing the new AbuseFilterRunner class easier (and
that will clean signatures, too).

Depends-On: I5b29ff556eca45fe59d15e2e3df4d06f1f6b3934
Change-Id: I7c1ea17adf7f42cf9260d416906bfbf3b8a20688
2019-06-19 15:14:17 +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 246d8e78aa Improve getFirstFilterChange function
Fix the typehint, and use selectField instead of selectRow.
Follow-up of Ie550889495232b534c0f9aec31039cf21b2135b1 suggested by
Krinkle.

Change-Id: I7e74b7b484dfa487db96598ef7aef4895d7bf275
2019-06-17 13:01:56 +02:00
Daimona Eaytoy e7cd4b2a98 Rewrite AbuseFilter::decodeGlobalName
Now it returns an array with a bit more info, and has a different name
to reflect the fact that its input is now split in two parts. Plus, make
it throw whenever it gets an unexpected input, and add a bunch of test
cases for it.

Depends-On: Ib5fdeb75c1324f672b4ded39681f006fde34b4d1
Change-Id: Ie550889495232b534c0f9aec31039cf21b2135b1
2019-06-12 23:56:25 +00:00
Thalia 22ceae7e23 Use MediaWiki\Block\DatabaseBlock instead of Block
This follows the rename of the Block class in I6d96b63ca0.

Change-Id: I44cf9eb68c23a8299316effa4dee7f732486dd84
2019-05-31 16:08:19 +01:00
Daimona Eaytoy 53f03e5301 Tokenizer caching back to APC
Partial revert of I4dd81a723e2bdb828b90594ad66a3918d8ec5b6c.
Thinking again of it, I think it's not worth it to have this data over
the network. Plus, given that it's not-that-slow to be computed, I think
there can only be a performance gain in using APC (as opposed to e.g.
memcached/redis) for 99.9% of the filters.

Change-Id: I8c6a4a95ec12c18ede8e6419540f7a2ac943457c
2019-05-28 19:48:26 +02: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
jenkins-bot e5a15ab968 Merge "Add a parameter to generate(User|Title)Vars hooks to specify context" 2019-05-25 11:37:04 +00:00
jenkins-bot 2b1c62ecdd Merge "Restore unit tests for CachingParser and fix it" 2019-05-25 11:24:51 +00:00
Daimona Eaytoy 2b4ddd1096 Change a long if/elseif to switch
This is done for 3 reasons: 1-the code should hopefully be clearer;
2-FWIW, switch's are a little bit faster than elseifs (roughly 15%); 3-to
fix a bug with coverage driver which says those lines are not covered.
3 is a follow-up of I997576141943959d4602a9f839311108928ec766.

Change-Id: I2d69e421e384cb74a799c5c5f77d041a7e02d4c8
2019-05-25 10:59:37 +02:00
Daimona Eaytoy 39fc7c12af Restore unit tests for CachingParser and fix it
Added cachingParser back to *all* the parser tests, fixed a couple of
differences with the normal parser, and added a couple of tests so that
any cachingParser-related file has 100% coverage. Also move the remaining
get_matches tests inside parserTests, and specify the parser used in case of failure.
This also adds a new base class for parser-related tests with a couple
of util methods.

Bug: T201193
Change-Id: I980aec3481a52ecc35f1811a366014a5581a7cdb
2019-05-25 10:55:24 +02:00
jenkins-bot 1cb80be0ad Merge "Add tests for various data type casts" 2019-05-24 19:19:20 +00:00
jenkins-bot 058e215882 Merge "Refactor tokenizer caching" 2019-05-24 19:09:03 +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
Daimona Eaytoy a766e39ade Add unit tests for profiling
Yet another important part to have covered. While for normal edits it
already works, for stashed ones it doesn't. That's why we need the patch
for checkAllFilters. Since for stashed edits profiling stats are all
zeros, this may explain T201334.
Changed the timestamp variable to use wfTimestamp instead of time() so
that we can fake it inside unit tests.
In a subsequent patch we should add average runtime conditions to tests
(really tricky).

Bug: T201193
Depends-On: Ib17821240b25c972a187e6b5eae42c5ada6c65e7
Change-Id: I5ee7ba44a6cd82a5ddb24fb4127af04d96e647f4
2019-05-23 08:47:40 +00:00
Daimona Eaytoy 33fca4e096 Ignore trailing commas in function calls
First step before removing this weird syntax. I'd love to add a unit
test for params count, but I couldn't find a way, since doLevelFunction
is protected, relies on class members, and the args count is local.

Bug: T153251
Change-Id: Ica3e49f5b00595a95513d9683732e490aa7aae17
2019-05-21 23:13:31 +00:00
jenkins-bot a9afdc1f80 Merge "Slightly improve degroup action, remove Phan suppressions" 2019-05-19 17:20:23 +00:00
jenkins-bot 48ac8c492b Merge "Temporarily catch another BadMethodCallException" 2019-05-19 17:13:10 +00:00
jenkins-bot e66d30d37c Merge "Don't send empty array to Database::makeList" 2019-05-18 12:27:50 +00:00
jenkins-bot 75e5c907fc Merge "Remove problematic array type hint from hook handler" 2019-05-18 09:01:48 +00:00
Daimona Eaytoy 291c35cea0 Don't send empty array to Database::makeList
Check that the provided param is not empty, as otherwise
Database::makeList will throw and the exception will bubble up to the
user.

Bug: T222531
Change-Id: Icf5db25037a0d0a7b4076f21e7f1c9a6ee1d5a87
2019-05-18 10:55:26 +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
jenkins-bot 6250eaea39 Merge "Add missing type hint to SpecialPage::execute()" 2019-05-16 15:19:22 +00:00
Thiemo Kreuz aba489a1f4 Add missing type hint to SpecialPage::execute()
[Also make use of the list() feature in one case I forgot before in
If2b6c95.]
 -> Changed to use direct array access by Daimona per inline comment.

Change-Id: I708dff30b6e00ccab3257b2e6fa5995eb9e30e0f
2019-05-16 14:31:54 +00:00
Daimona Eaytoy 44632f21a4 Temporarily catch another BadMethodCallException
This is the same as line 224, and I forgot to include this code path in
the same patch.

Bug: T187153
Change-Id: I28074680760a7070eb423b5eada1e35f829ed10a
2019-05-16 15:49:17 +02:00
jenkins-bot 99e821b125 Merge "Upgrade PHPCS and phan" 2019-05-15 19:42:42 +00:00
Daimona Eaytoy 311f68f6e8 Upgrade PHPCS and phan
Change-Id: Ibfb737e4552133d1ddd388e7410c6e1bc3a72e12
2019-05-15 19:06:04 +02:00
jenkins-bot 915bea466e Merge "Make use of PHP's list() feature where possible" 2019-05-15 15:06:20 +00:00
jenkins-bot c52850aae7 Merge "Add missing limits to explode() calls" 2019-05-15 15:06:18 +00:00