Commit graph

672 commits

Author SHA1 Message Date
jenkins-bot fea7686724 Merge "Hide profiling for disabled filters" 2019-08-04 16:12:56 +00:00
jenkins-bot f7fd6a6daf Merge "Move per-filter matches profiling to per-filter data" 2019-08-04 16:07:58 +00:00
jenkins-bot d940ef63cd Merge "Specialize empty AFPData types" 2019-08-04 15:52:34 +00:00
Daimona Eaytoy 9049be3609 Specialize empty AFPData types
As described in T156096#5389655.

Change-Id: Ifbf95a6b72a280cd77db6affbd8d642499bbfedc
2019-08-04 15:26:57 +00:00
Daimona Eaytoy c8ebb4956c Hide profiling for disabled filters
As data could be "old" and it may have no meaning.
Also remove a superfluous isset(), as $row->af_hidden is always set.

Depends-On: I2d8c8f8278073a9420e3eb373fb89a655925618a
Change-Id: I072363706c61f272c4c3691de4078e2a19148424
2019-08-03 23:28:42 +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
jenkins-bot 3b2b85b60d Merge "Store per-filter profiling in a single key" 2019-08-03 22:43:32 +00:00
Daimona Eaytoy 33bfe97d8c Move non-decimal numbers deprecation logging
Bug: T212730
Change-Id: Idb833c60541873bfe9c2b225009bd32e4a48cd60
2019-08-03 16:57:24 +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 a85e1ccc59 Make AbuseFilterParser::$funcCache non-static
Change-Id: I312efe3ce4d1f06e697aa4564aeec1bacbaf97d3
2019-08-03 09:19:49 +00:00
jenkins-bot 0e00654b7d Merge "Save profiling data and vars in cache when running filters" 2019-08-02 23:28:03 +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 09d0254172 Better handling of DNONE
This patch includes:
 * Making it possible to access offsets of a DNONE (returning a DNONE)
 * Initializing user-defined variables as DNONE inside short-circuited branches
 * Make DNONE propagate with other operators
 * Make DNONE count as false for logic operators
 * Remove a now-outaded bit in doLevelAtom. In case of shortcircuit,
   $result is now DNONE instead of DNULL, and thus it's possible to
   access offsets of it. Performance++!
 * Don't allow modifying or adding an element of a DNONE as if it were an
    array (to avoid inconsistencies)

This re-applies Id85c673337fa90a3782fd22eb9690cd996967111 with several fixes.

NOTE: Haven't tested locally, although I'm pretty confident thanks to
the amount of tests added.

Bug: T214674
Bug: T228677
Change-Id: I5ec4ab44c4e88aaf18c0d7b73355d27050beeda7
2019-08-02 21:05:08 +00:00
jenkins-bot e3e157361d Merge "Revert "Initialize user-defined variables during shortcircuit"" 2019-07-29 23:30:50 +00:00
Daimona Eaytoy 13cdb86dd2 Revert "Initialize user-defined variables during shortcircuit"
Reason for revert: T214674#5374806

This reverts commit 56e6117afd.

Bug: T214674
Change-Id: Iccce248d2693cd9877a740b74e72a577e730435e
2019-07-29 23:06:23 +00:00
jenkins-bot dfa0109ba8 Merge "Rename old/new-(wiki)?text i18n keys" 2019-07-25 08:35:26 +00:00
Daimona Eaytoy eff4580a6f Add new method: AbuseFilterVariableHolder::newFromArray
Instead of duplicating code in several files.

Depends-On: I2eab2e50356eeb5224446ee2d0df9c787ae95b80
Change-Id: Iafc142d2e5ba7aa0fb0d3265fa05cace27679738
2019-07-24 18:41:32 +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
Daimona Eaytoy 56e6117afd Initialize user-defined variables during shortcircuit
Bug: T214674
Depends-On: I5a14d4b2bc3ffd9caaaa095f16f36b9b6009db05
Change-Id: Id85c673337fa90a3782fd22eb9690cd996967111
2019-07-23 12:20:53 +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 18d7d2ed62 Start using AFPData::DNONE
This should allow more flexibility when checking syntax, and a saner
behaviour overall.
Aside from not throwing exception in certain cases, the results should
be almost equal to the ones you would get without this patch. However,
there are still a few things to improve (which for convenience I wrote
inside the parser test) and many to test.

Bug: T204654
Depends-On: I69bfec45c76509fb1112641393f78e8d8834adcd
Change-Id: I5a14d4b2bc3ffd9caaaa095f16f36b9b6009db05
2019-07-14 08:48:47 +00:00
jenkins-bot d36dfaa951 Merge "build: Upgrade phan-taint-check-plugin from 1.5.x to 2.0.1" 2019-07-10 16:41:46 +00:00
James D. Forrester 70a03755e8 build: Upgrade phan-taint-check-plugin from 1.5.x to 2.0.1
Change-Id: Ica0439db5ec729c3b298db99fd89dd999f491457
2019-07-10 15:30:52 +00:00
Daimona Eaytoy 7bc566e635 Fix the regex for numbers, start deprecation of non-decimal numbers
Aside from the 14 thingy reported in the task, this syntax is awful! The
fix to the regex should only be intended as a temporary stopgap. A
proper fix would be to introduce a new syntax, like for instance the one
used in PHP.

Bug: T212726
Change-Id: Idc37a17ce539e6c63d67fc07d47d812569debe0e
2019-07-10 13:26:36 +00:00
Aaron Schulz fce600c3ee Fix bogus DB domain parameter in AbuseFilter::getCentralDB()
Follow-up to 2cf7b58434

Bug: T227613
Change-Id: I07b2d46389e6c8346d7c5848a00a1c2f8577acd8
2019-07-09 15:27:53 -07:00
jenkins-bot 6a5d5fc447 Merge "Really drop afl_log_id from update.php" 2019-07-09 17:03:24 +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
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 db193dad12 Rename old/new-(wiki)?text i18n keys
Now we have the key old-text for the old_wikitext variable, and the key
old-text-stripped for old_text. This can be confusing (see I61b2d252333ca634eae560d824f740f0f947b3d3), so use i18n keys more similar to the variable name.
NOTE: the keys will have to be changed on translatewiki if we want to avoid
confusing people.

Change-Id: Ie612350642ac4afc76f18639d988e72b4016b1e2
2019-07-08 15:55:02 +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