Commit graph

231 commits

Author SHA1 Message Date
Daimona Eaytoy e5746bbb0e parser: Add a BC option to get DNULL for unset variables
While checking a filter, if a variable is not set (e.g. added_lines for
an account creation), the VariableHolder will return a DNULL, rather
than a DUNDEFINED. This means that some filters will resume working, and
the WMF servers will stop getting AF warnings at a rate of 4 millions per
day. This also requires adjusting some tests to reflect the new
behaviour (which is actually the OLD behaviour, that filters had until
last year when we introduced the DUNDEFINED data type). It also requires
adjusting a check in the old parser, but that's not really relevant
because the plan is to remove the old parser before 1.36 is released
(see I0e75f334c7e0dfc1239f2e5f5f7d7452b0bbf29e).

Bug: T230256
Change-Id: I4d06303047397674c1edbfc32628f1bc83ac3340
2020-09-18 15:05:58 +02:00
Daimona Eaytoy 5d2eaa052c Revert "Fix a test which will be broken by Hooks::run() migration"
This reverts commit be3fbdf347.

Reason for revert: The commit mentioned here was merged a few months ago, so this temporary workaround can now be reverted.

Change-Id: I1e8b4a23b700a2b566c087b8a3ea2229c95bcc3f
2020-09-16 07:06:06 +00:00
Tim Starling be3fbdf347 Fix a test which will be broken by Hooks::run() migration
Temporarily mark the test skipped on earlier versions of core, to avoid
a circular dependency which blocks the merge.

Change-Id: I5db9937f249edaf7c070b2436c0caea369dac8ef
2020-05-05 11:11:54 +10:00
jenkins-bot b118fd50dc Merge "Improve var dumping in /details, /examine and /tools" 2020-04-29 20:00:54 +00:00
DannyS712 1b65bd1862 Remove a remaining use of Revision objects
Remove use of Title::getFirstRevision and Revision::getUserText

Bug: T249393
Bug: T250579
Change-Id: I0f77b124a0c7de1dec6baf4c997e0997ecdd55f8
2020-04-23 18:39:20 +00:00
Daimona Eaytoy 1d6b9f6617 Add new methods for checking DUNDEFINED recursively, use them
The problem is explained at T250570#6068702; basically, the previous
check didn't account for DUNDEFINED nested deep inside arrays.

Bug: T250570
Change-Id: Iacee2db54ca00108de6339bb3dae70af7e2eeb56
2020-04-19 13:58:14 +02:00
Aaron Schulz 904da3d080 Remove references to "abuse_filter_actions" from tests
Change-Id: Idfe61e8f2654ee9953d37ce84a9e10f622b23ca4
2020-04-06 16:06:15 -07:00
Daimona Eaytoy 4c98aecf4d Improve var dumping in /details, /examine and /tools
Using var_export for better visual effect, especially for arrays.
The result from /tools is much clearer and the 'wrong syntax' message is
a bit more explicative than before.

Bug: T190653
Bug: T239972
Change-Id: I79a17305c7f19f7900f896f895e9365bb5f2fd58
2020-03-28 17:35:43 +01:00
DannyS712 57f85ed532 Pass a user to WikiPage::doDeleteArticleReal, use new signature
Don't need to worry about supporting prior versions, since AbuseFilter
requires 1.35+

Bug: T247869
Change-Id: I112e929dcdd9edcf3ca433b75356659a3179bbcd
2020-03-19 00:44:57 +00:00
Daimona Eaytoy 2c03c77d9f Add a maintenance script to clean afl_var_dump
This script aims to fix every problem reported in T213006. Subsequent
patches will add new code and drop the back-compat one.

Bug: T213006
Bug: T187153
Bug: T204236
Bug: T187731
Bug: T204235
Bug: T214193
Bug: T214196
Bug: T34478
Depends-On: I5b29ff556eca45fe59d15e2e3df4d06f1f6b3934
Change-Id: I22cf698c5be77506727cbd227c67e037a5d89b5c
2020-02-28 19:41:30 +00:00
jenkins-bot 08c0a3f482 Merge "ViewEdit: add af_id to the row" 2020-02-26 16:53:35 +00:00
jenkins-bot b28e9e8c1f Merge "Start using new format for var dumps" 2020-02-26 16:51:02 +00:00
jenkins-bot 22adab7159 Merge "Stop using the Revision class" 2020-02-26 16:47:37 +00:00
Daimona Eaytoy f454e60e83 Start using new format for var dumps
Migrating old log entries is I22cf698c5be77506727cbd227c67e037a5d89b5c.

Bug: T213006
Change-Id: I3242acd5c5163a941f584d6119e3ad3b3cad8c29
2020-02-26 16:03:38 +00:00
Daimona Eaytoy 518c176754 Stop using the Revision class
Change-Id: Ie257c9b1ea94dcadce59f4541d5947465262bd75
2020-02-26 15:39:12 +00:00
Daimona Eaytoy fe28aff82a ViewEdit: add af_id to the row
A PHP notice is sporadically emitted in production, e.g. reqId XlVEMgpAMNAAA6zMVhQAAACV

Change-Id: Ie42d00c6520aa31daf127c5df9515a3ab01d986f
2020-02-26 15:27:54 +00:00
Daimona Eaytoy b9a1e86245 Remove old number syntax
Bug: T212730
Change-Id: I7573da1683efc83b5002b8948c97dd7f6658a488
2020-02-25 23:38:19 +00:00
Daimona Eaytoy 1bac110205 Remove dependency on $wgRestrictionTypes
This was used to dynamically generate *_restriction_* variables.
However, it had two big problems:
 - We only have i18n for 'create', 'move', 'edit', and 'upload' (the
 default value of the global); other restrictions would show missing
 messages in various pages.
 - We had to access the global state in various points.

This change also makes some code in AbuseFilterVariableHolder simpler,
and also allows us to make AbuseFilterTest a unit test.

Change-Id: I321ad6e07f8243200af67a581b6e485970efd3ce
2020-02-25 23:17:54 +00:00
Daimona Eaytoy bef72c7dc3 tests: Use ChangeTags::getTags instead of hardcoded queries
This is the "clean alternative" mentioned in the method comment!

Change-Id: Ieb87a1f512c930c2e33e721ba792986bc198e414
2020-02-22 13:54:23 +00:00
jenkins-bot 76a1be97a4 Merge "Add site name and language variables" 2020-02-10 19:06:01 +00:00
Daimona Eaytoy 0d2cab0deb Validate imported data
At the moment there's no validation for import data, so it's totally
possible to insert rubbish in the field, and the code will produce other
rubbish. For instance, it's not so uncommon to see lots of PHP notices
on logstash for ViewEdit code trying to access members of the imported
data as if it were an object.

Change-Id: If9d783f0f9242d3d1bc297572471e62f51ee0e40
2020-02-10 18:41:36 +00:00
Daimona Eaytoy d9ae71f578 Add site name and language variables
In T43172 it was told that adding the site name could increase the risk of
attracting more spam, but I don't see how this variable could cause that.

Bug: T240948
Bug: T97933
Change-Id: I1d2aeabaf008ac06798b8d7e4af7d61ae1702776
2020-02-09 14:32:02 +01:00
jenkins-bot c0b58d7699 Merge "Factor out variables-related methods" 2020-02-08 14:42:13 +00:00
jenkins-bot 430058f2c0 Merge "Avoid keeping superfluous row properties" 2020-02-07 21:26:34 +00:00
jenkins-bot 02cd866f53 Merge "Refactor data load in ViewEdit" 2020-02-07 21:26:32 +00: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
Daimona Eaytoy 1686042a91 Move variable generators to new classes
RunVariableGenerator is for generating variables based on the current
action;
RowVariableGenerator is for RC entries;
VariableGenerator is the generic one.

This patch only moves the methods to the new classes, to keep the diff
easier to read, and facilitate conflict resolution. These classes will
then be revamped in I366d598b69ad866496b7cb0059e0835c02e54041.

Note that these classes are now namespaced.

One method, AbuseFilter::getEditVars, was renamed to
AbuseFilterVariableGenerator::generateEditVars, because it would
otherwise conflict with an incompatible method in RunVariableGenerator.

Change-Id: Iff412e5492873d4fae55402939a51609e64d55a8
2020-02-07 19:44:31 +00:00
Daimona Eaytoy 472d1221bd tests: Increase and rebalance code coverage
Also fix a couple of broken tests in Consequences:
 - For createaccount, $user->addToDatabase must be called before
   testForAccountCreation, or it will throw a CannotCreateActorException.
 - In testThrottleLimit, also set wgAbuseFilterEmergencyDisableThreshold
   to avoid relying on the local config.

Bug: T201193
Change-Id: If1a50b0a729e4d554485f2e2225d5877510966b6
2020-02-07 18:32:17 +00:00
Daimona Eaytoy 102789f62a Avoid keeping superfluous row properties
Most of them are overwritten either in ViewEdit::loadRequest or
AbuseFilter::saveFilter. af_hit_count and af_throttled are actually
relevant for the old version, so list them explicitly. And also add
default af_group and af_global, which are later read, for import action.

Depends-On: Iabd0ae5b18571f8cad44ef2d86bcf2519e7f95ba
Change-Id: Ie9aae938cca06e38a7a834a3f74f3e8735ab01ee
2020-01-23 12:50:03 +00:00
Daimona Eaytoy 53b9f38888 Refactor data load in ViewEdit
Instead of having a single loadRequest method (which could end up
loading from the DB...), split it in a DB-only method and a request-only
one. Simplify the logic used to show the filter editor. Show the page
without changes or warnings if the user lost editing rights in the
meanwhile. Avoid two static properties, and pass them in when relevant
instead. Bonus: optimize a query to sort by afh_id instead of afh_timestamp to avoid filesort.

This will allow a subsequent patch to clean the $row object in
loadRequest.

Change-Id: Iabd0ae5b18571f8cad44ef2d86bcf2519e7f95ba
2020-01-21 14:15:41 +01:00
Daimona Eaytoy e9fe252def Fix remaining PHPCS issues
Mainly, add visibility modifiers on constants.

Change-Id: I41e8e2d691b2bad6ea6f244d54517d37d7783181
2020-01-21 12:36:37 +00:00
Max Semenik 8e7230076e Fix PHPUnit 8 warning
Bug: T192167
Change-Id: Ifbebbc3467eb0bf3f12cffc9e5601a1c94327bd9
2020-01-20 15:47:45 +00:00
Daimona Eaytoy 44ea3aa7f4 Fix generation of HTML vars, simplify tests
-new_html: also strip the "Transclusion limit" comment if present, and
anyway take it into account (as well as a "</div>"), which right now
prevent the PP limit report from being stripped as well.
-new_text: trim extra whitespace on the right, which is created when
stripping the aforementioned comments.

Also simplify the test for getEditVars, make it not blindly copy what
AFComputedVariable does.

Extra: kill a temporary variable.

These changes are partly taken from
I96785c6c5fdf381c21d5f8930ee12e706abb7f3f.

Change-Id: I2b4c84a3d9d0d17ce229088197b75781d5181b4f
2020-01-12 17:44:02 +00:00
jenkins-bot 5c9fe8bd9b Merge "Always evaluate the offset when retrieving array elements" 2019-12-27 09:58:50 +00:00
Daimona Eaytoy 8ad4ecd31d Always evaluate the offset when retrieving array elements
Even if the array is DUNDEFINED, we need to check the offset to ensure
that it's valid.

Bug: T237351
Change-Id: Ibfa360c4ae1d80abe14d9fdf66991b76cb5954df
2019-12-23 16:04:45 +00:00
jenkins-bot ce85c215f4 Merge "Ensure that a min/max arg count is available for all built-in functions" 2019-12-23 12:11:57 +00:00
Daimona Eaytoy b3e0529d55 Log deprecated vars in the cached phase in the new parser
For the new parser, xhgui shows that AbuseFilterParser::getVarValue is
taking up a lot of time; in turn, most of the time spent inside
getVarValue is used to log the use of deprecated variables. Hence, given
that:
 - We should keep the new parser performant
 - There are tons of deprecated variables out there and they likely
 won't be replaced
 - Having gazillions of debugLog entries doesn't help

log them only in the cached phase.

Bug: T234427
Change-Id: I2bfc692c829c3cbe889e5076f5205e2c99097087
2019-12-16 13:54:58 +01:00
Daimona Eaytoy f382304aae Add a base class for parser transition
Change-Id: I31282b8632c332b6d46a6bb4a42f57ac0d005b5f
2019-12-15 13:29:56 +00:00
Daimona Eaytoy 07572da2fe Really throw for too many params
Bug: T230803
Change-Id: I4e68bb7220f1151bb32b2be859f6cffc55888a30
2019-11-30 10:57:16 +00:00
Daimona Eaytoy 2ddd79fd98 Forbid assignments where the LHS is a built-in identifier
And not just a built-in variable.

Bug: T237130
Bug: T237216
Change-Id: Ie1d86dc324993efcb863be23697732e6aa1dac10
2019-11-28 14:40:38 +00:00
Daimona Eaytoy 0672a8eb8a Ensure that a min/max arg count is available for all built-in functions
This is especially useful for old patches, created before the
introduction of FUNC_ARG_COUNT, where a rebase may break the parser.

Change-Id: Ib142438626a7305f102dc3e4cc9cb07ad33902b8
2019-11-22 17:59:00 +01:00
jenkins-bot a8c50150d6 Merge "Convert static arrays to constants" 2019-11-22 13:39:39 +00:00
Daimona Eaytoy b3e58067ac Set the utf-8 flag for var dumps in the text table
This is not retroactive; that will be handled as part of T213006.

Bug: T34478
Change-Id: I2c532da71719a9ace1279bbf67d6e6e30e9a986c
2019-11-16 16:00:45 +00:00
Daimona Eaytoy c03f0a3b08 Convert static arrays to constants
Beloved PHP7!

Change-Id: Id5170662f7c5ceacfc0ac8d90787f2c92fd93464
2019-11-16 16:32:36 +01:00
Daimona Eaytoy a77a59b962 Hard-deprecate empty operands
This bumps the level to WARN, and makes it very clear that people should
fix the affected filters. It also removes the calling method, which was
mostly meant for debugging purposes, and changes the type to 'op_type'
to avoid conflicting with type:mediawiki in logstash.

Bug: T156096
Change-Id: Ie73f1604e8ed82bc2e1be9fc90fa065be37889a3
2019-11-12 11:39:25 +00:00
Daimona Eaytoy c0f8374624 Check for 0-like floats passed to the modulo operator
That throws an error in PHP.

Bug: T237459
Change-Id: Ia0b29d6a8b9f4aac6b5b72ce8f2f45afb03f4c99
2019-11-10 11:22:04 +00:00
Daimona Eaytoy 585d6cdb24 Make to sure to report division by zero when the LHS is undefined
Bug: T234339
Change-Id: I1575ec013c1e7e321a8f13f40804ebc5ab076268
2019-11-08 14:08:52 +00:00
Daimona Eaytoy 1abaff1aac Better handling of keywords and functions
Always run the keyword/function handler, even if there are DUNDEFINED
arguments, so that the handler can perform further validation on the
input and report any error to the user. However, replace DUNDEFINED with
DNULL before running the handler, to avoid special-casing DUNDEFINED in
every handler. If any argument was a DUNDEFINED, we will return
DUNDEFINED anyway.

Also centralize the keyword handling logic to a new method, like it
happens for functions.

Bug: T234339
Change-Id: I875cb77418a39790e91fe5867c49917bfe406ed4
2019-11-08 15:07:20 +01:00
Daimona Eaytoy b7c7ae168d Explicitly forbid negative indexes in arrays
This emits its own error because:
1- It's clearer to understand
2- It's easier to find where we're dealing with negative offsets, if
we'll ever want to allow that.

Note that trying to use a negative index already results in a hard PHP
error being thrown.

Bug: T237219
Change-Id: Ib11eaaca5e21f740269141c75e62bac48093e8d0
2019-11-08 05:55:56 +00:00
Petr Pchelko 915b9a1538 Remove usages of deprecated User methods
Bug: T220191
Change-Id: I54e20870a32ff98b41a98495694ff563c4c4c5ca
2019-10-30 12:51:01 +00:00