Commit graph

790 commits

Author SHA1 Message Date
jenkins-bot 534ad9cfe6 Merge "Add a service to format filter specs" 2020-12-20 10:21:39 +00:00
jenkins-bot 16ef710ee3 Merge "Test some Consequence classes and clean up" 2020-12-20 09:25:39 +00:00
Matěj Suchánek 63b950e5b6 Test some Consequence classes and clean up
Sadly, these are not unit tests.

Bug: T201193
Change-Id: I4c977ab14b273b02803a63f0a7b152a581a838b2
2020-12-19 16:31:22 +01:00
Matěj Suchánek 352a207c70 Improve code coverage
Bug: T201193
Change-Id: Ie086fd525bec19c63c13f8710a27897229cc33c8
2020-12-19 16:28:34 +01:00
Daimona Eaytoy 7bcb5ec2d5 Add a service to format filter specs
This requires a MessageLocalizer, which currently means providing the
main RequestContext. This is the only alternative right now, until core
provides a proper MessageLocalizer service (see T247127).

Change-Id: I8c93e2ae7e7bd4fc561c5e8490ed2feb1ef0edc2
2020-12-19 14:22:08 +01:00
Matěj Suchánek dc7509811a Notify of a throttled filter
Use Echo for delivering the notification to the last
user who edited the filter.

Much boilerplate.

Change-Id: I7a46a03b4f15de20902ec70c62fb4fe750096842
Depends-On: If585b14a6dd6fb8c7d2c3bee1f20d9d08eaac706
2020-12-19 10:31:29 +01:00
jenkins-bot d386633103 Merge "Create a dedicated namespace for all consequences-related classes" 2020-12-18 22:57:00 +00:00
Daimona Eaytoy b394956c22 Create a dedicated namespace for all consequences-related classes
Change-Id: Ibc39593e34da36e57b640af0b5bbf2145f725e92
2020-12-18 19:27:33 +00:00
Daimona Eaytoy 7c1d1c6d7d Return warnings from the parser, add warning for catch-all regexps
This commit introduces some boilerplate for emitting warnings from the
AbuseFilter parser, and also code for showing these warnings in the ace
editor. Adding new warnings should be as simple as appending to
AbuseFilterParser::warnings (and adding the relevant i18n).

Bug: T264768
Bug: T269770
Change-Id: Ic11021b379f997a89f59c8c0572338d957e089a6
2020-12-18 18:22:41 +01:00
jenkins-bot e1a7acf55f Merge "Implement GetAutoPromoteGroupsHook" 2020-12-18 13:41:28 +00:00
Matěj Suchánek 2da4cb4321 Implement GetAutoPromoteGroupsHook
Test coverage included.

Bug: T261067
Change-Id: Iba49f0518d2c1a72405498b2743e5ef300dd6d61
2020-12-15 18:01:04 +01:00
Daimona Eaytoy 3f7dd25fbf Create FilterRunnerFactory
Next step is splitting the Runner into various subclasses.

Change-Id: I766555f31b425cee52fd262c5bfb1c73f3f170d2
2020-12-15 12:47:34 +00:00
Daimona Eaytoy 68adaa5cb1 Introduce ConsequencesExecutor
This is the last big step towards moving Consequences-related things away from
AbuseFilterRunner. There's still some cleanup to do (+ write proper tests), but
this should really be the last important code change.

Change-Id: I347795fe93ba496c43b1d5cfc9ba6e1326842c06
2020-12-15 13:47:21 +01:00
Daimona Eaytoy c52ef337d7 Add a VariablesBlobStore service
Change-Id: If0c1eab2391819f8b4c801d12275d9ec14490f7a
2020-12-15 02:35:15 +00:00
daniel dfeff89317 Use a BlobStore for storing var dumps
AbuseFilter emulates the storage mechanism also used for page content.
Instead of duplicating the relevant code, AbuseFilter should use the
same BlobStore service also used by RevisionStore.

Note that this change is not strictly needed to resolve T198341, but is
needed to unblock T183490

Bug: T261889
Bug: T198341
Bug: T183490
Change-Id: I3fc8475dd8d50d73d705b706ff597a130267e990
2020-12-15 02:35:05 +00:00
DannyS712 7ccf758c4b AbuseFilterConsequencesTest: stop setting $wgUser
Shouldn't be needed anymore, not read by extension

Bug: T246733
Change-Id: I10dc21ad34402d83d57f23bc754a437b8a015af7
2020-12-14 20:53:00 +00:00
Daimona Eaytoy 9d288478fe Remove a bunch of deprecated methods
These are all unused.

Depends-On: Id99da02a98bb392cafed370768edcc8ac3d712ab
Change-Id: I9ff17714f94e1fbbd52da32ebf4f054a551edc1f
2020-12-13 18:31:27 +00:00
Daimona Eaytoy 5e609eb537 Add GlobalNameUtils class
This is just a temporary location for these two methods. Since they're
used a lot, having them in the AbuseFilter class means that the
dependency graph is unnecessarily complicated. Thus, since these methods
aren't doing much, they were moved to a dedicated class. Future todo is
finding an appropriate location, that might be either as part of another
service, or keep them in a Utilities class, perhaps a single class with
all util methods, rather than a specific class.

Change-Id: I52cc47a6b9a387cd1e68c5127f6598a4c43ca428
2020-12-12 17:49:48 +00:00
jenkins-bot 93c477d4b8 Merge "Move parser classes to a dedicated namespace" 2020-12-09 10:48:36 +00:00
Daimona Eaytoy da1c71ec4c Move parser classes to a dedicated namespace
Names were kept for now.

Change-Id: Ib2eb5d7b523a64f2a0f72fdcdde2043a76cc9a37
2020-12-09 01:30:20 +00:00
Daimona Eaytoy ca3f652cd7 Almost kill the last use of wgUser
This is the last use, and it was a bit harder to remove because it was
buried inside AFComputedVariable. Starting with
I4444cada720ab62d187f2dd0c4760697e465f2ff, we can freely change the
parameters to AFComputedVariable without breaking old log entries.

Note, we still need a fallback for other extensions calling this
method...

Bug: T246733
Depends-On: I4444cada720ab62d187f2dd0c4760697e465f2ff
Change-Id: I5d786a518ef88fad9c8d9c25ef4553a0bf30b2b2
2020-12-08 23:28:24 +01:00
Daimona Eaytoy 815ef6051c Split afl_filter in afl_filter_id and afl_global
Add a script to migrate the columns (which can also
be executed in dry run), and a config option with the migration stage
(defaults to SCHEMA_COMPAT_OLD).
Some of the script-related code is stolen from
Ic755526d5f989c4a66b1d37527cda235f61cb437.

Bug: T220791
Change-Id: I7460a2d63f60c2933b36f8383a8abdbba8649e12
2020-12-08 18:31:27 +00:00
Daimona Eaytoy 1c625eeae4 Drop back-compat code
This should be merged once T246539 is done.

Bug: T213006
Change-Id: I4444cada720ab62d187f2dd0c4760697e465f2ff
2020-12-08 17:15:47 +00:00
Daimona Eaytoy 600f1735f2 Add a hook for extensions to add custom actions
The global is now deprecated, and it will be removed soon.

Bug: T265794
Change-Id: I4e6c9b143744cb72c441017921bac9cd1960609c
2020-12-04 16:10:44 +00:00
Daimona Eaytoy d351d7150b Always take into account custom actions
$wgAbuseFilterActions shouldn't be used normally, as it excludes actions
registered by other extensions.

Note: mw:Extension:AbuseFilter#Integration_with_other_extensions should
be updated after merging.

Bug: T239348
Change-Id: I89b3f0228eacdf145e8f2dd2a5602d0c7ce75a86
2020-12-03 21:39:35 +00:00
Daimona Eaytoy c786c4adc6 Add ConsequencesRegistry
Change-Id: I91f4f28e09fa46b9ab2457b2a241b6f105320bdd
2020-12-03 22:39:25 +01:00
jenkins-bot d02b10db97 Merge "Improve type safety of filter ids" 2020-12-03 17:50:31 +00:00
Thiemo Kreuz 34968d783e Simplify a few pieces of code
… mostly by inlining pieces, instead of assigning them to
a variable first.

Change-Id: Ibc432ed05f7b853a44fc1a301ef820984facb067
2020-12-03 16:50:55 +01:00
Matěj Suchánek de997fe98e Improve type safety of filter ids
Also fix a bug in FilterProfiler. It would attempt to reset
stats for global filters but we do not record them (yet?).

Change-Id: I0228d8c85dab146deb877dfce506f1e8e7711a9f
2020-12-03 14:58:51 +01:00
Matěj Suchánek 0f062fca06 Move AbuseFilterView classes to separate namespace
Change-Id: I569281b13ec81d9f35038c7ef17a2d98f16f9b5c
2020-12-03 13:06:53 +01:00
Matěj Suchánek a1ba43f586 DI for AbuseFilterView and subclasses
Also add a test to ensure all views can be instantiated without problem.

Change-Id: Iedd7a5dca240efab1077fa51a3522c983b0ba4fa
2020-12-03 13:04:35 +01:00
Thiemo Kreuz e17e1b7e01 Remove comments that literally repeat what the code says
Such comments don't add anything.

Change-Id: I7530d6693293fbdd06ca3ee077c6e783fd9a4ac1
2020-12-03 09:50:56 +01:00
Daimona Eaytoy c957188866 Add ConsequencesLookup
The class is used to retrieve consequences from the Database.

Change-Id: I46b3925aac47554723649c076eff64707a2ea2e6
2020-11-27 16:43:44 +01:00
Matěj Suchánek d76affb1db Move ChangeTags stuff to separate namespace
Change-Id: I6d7bed0e62f001f82c00a3528cc0018388c9c70e
2020-11-27 15:13:34 +00:00
jenkins-bot a6e96ed915 Merge "Introduce ChangeTagValidator service" 2020-11-27 15:13:06 +00:00
Matěj Suchánek 872b6118f4 Introduce ChangeTagValidator service
Just moving code around. Without a unit test because DI
coverage of change tags in core isn't available yet.

Change-Id: Iac861e1e24dae13581b8d9173357a1d6c94be88a
2020-11-27 15:11:48 +01:00
Matěj Suchánek 1ad77dc9fb Introduce EditBoxBuilderFactory service and EditBoxBuilder
It makes sense to look at this and Iedd7a5dca24 together,
as this patch itself doesn't really fix anything.

Change-Id: Ifef5266b1803d1a96489789b08d9beed044d908f
2020-11-26 14:49:04 +01:00
Daimona Eaytoy 904d9cddbb Represent Consequences with command objects
The consequence-taking logic is moved away from AbuseFilterRunner, to
dedicated classes. There's now one class per consequence, encapsulating
everything it needs to take the consequence.

Several interfaces allow customizing different types of consequences.
Every "special check" in AbuseFilter was generalized to use these
interfaces, rather than knowing how to handle each consequence.

Adding more consequences from other extensions will also be easier, and
it should happen via a hook (not a global), returning a class that
implements Consequence. The BCConsequence class was temporarily added
for legacy custom consequences.

A ConsequenceFactory class is added to instantiate consequences; this
would possibly benefit from using ObjectFactory, but it doesn't because
it would also reduce readability (although we might do that in the
future).

These classes are still not covered by unit tests, and this is left to
do for later. The new unit tests should mostly replace
AbuseFilterConsequencesTest. @covers tag were added to keep the status
quo (i.e. code that was considered covered while in AbuseFilterRunner
will still be considered covered), although we'll have to adjust them.

Change-Id: Ia1a9a8bbf55ddd875dfd5bbc55fcd612cff568ef
2020-11-25 17:35:36 +00:00
jenkins-bot 26d3abfab4 Merge "Introduce a service for saving filters" 2020-11-25 14:47:21 +00:00
Daimona Eaytoy c368575af0 Create a base interface for watchers
This will ease adding new watchers, for instance to send Echo
notifications (see T179495 and T100892).

For now, this is just boilerplate, and converting EmergencyWatcher to
the new interface.

Change-Id: I18d62aba53471202b709cdb19033b1729c5c25b4
2020-11-20 23:34:20 +01:00
Daimona Eaytoy 9595bd9da5 Introduce a service for saving filters
Change-Id: I6b7d16ad7ea1124989ed67c74413979cfd0275c4
2020-11-20 22:33:21 +01:00
Daimona Eaytoy 3f7fff56e8 Adjust code coverage
-Exclude methods and classes that cannot be meaningfully covered
-Add a simple test for AbuseFilterServices
-Exclude ServiceWiring because there's no way to tell PHPUnit it's
covered

Change-Id: I4c67b0d3fea68c7a3b3cbe01b5608f87e1b492db
2020-11-19 22:40:26 +00:00
Daimona Eaytoy eab1f13696 Make VariableGeneratorTest an integration test
It's actually using MediaWikiServices.

Change-Id: I6ec1b4723ff3f187eccf44a8b4ac286572fdfbbe
2020-11-19 13:55:16 +01:00
jenkins-bot 31f4607790 Merge "Handle DUNDEFINED in array offsets" 2020-11-18 23:30:58 +00:00
jenkins-bot 8f47259285 Merge "Add an interface for exporting/importing filters" 2020-11-18 23:13:53 +00:00
Daimona Eaytoy 3fc30021d2 Handle DUNDEFINED in array offsets
The behaviour is:
- When assigning to an undefined offset, delete the whole array and turn
it into another DUNDEFINED
- When retrieving from an undefined offset, just return DUNDEFINED.

Bug: T237214
Change-Id: I621ee7a16c90bb86a57be04e7ce0a748ecdbfcc7
2020-11-18 14:20:49 -08:00
Daimona Eaytoy 210cf29658 Add an interface for exporting/importing filters
The main benefit of having a dedicated interface is that we can easily
change the output format. So we're now using a custom array without
references to the DB schema, thus making the import/export process
completely independent from the schema.

Change-Id: I4c0de41d914baf1e9a0e588bd31f95b3524a424b
2020-11-18 22:06:09 +00:00
Daimona Eaytoy 7a24c94d6e Evaluate left-to-right when adding elements to array
Bug: T237090
Change-Id: I5fb72dec0ea12240b6563e66b69e399edc4c72d6
2020-11-18 21:25:45 +00:00
Daimona Eaytoy df017d478c Factor out another method from AbuseFilterRunner::getFilteredConsequences
This is a no-op, moving code around, introducing another distinction re
"filtering actions", which now happens in 2 steps:

 - The first step only uses "generic" information available by looking
   at enabled actions as a "group". This includes keeping only the
   longest block, and removing 'disallow' if other blocking actions are
   enabled.
 - The second step uses information that is only available after having
   "partly executed" (named "pre-checked") a consequence. For instance,
   we need to pre-check 'throttle' to see if the throttle was hit, and
   remove any other actions if not.

Change-Id: I7be5cfaa61e942a06f97ed52f50e9c8c70a120e8
2020-11-18 16:49:26 +00:00
Daimona Eaytoy ef9e828fbe Filter out actions to execute before actually executing them
This way we don't have special cases in executeFilterActions, and instead, we execute
all actions in the same place. In turn, this is going to ease the
transition to a new consequences system: next step is refactoring this
code into a service with proper DI etc.

Bug: T204447
Change-Id: I8134ecc41fbecdbed99faf406e9e3ca91b6123b9
2020-11-18 16:49:01 +00:00
Matěj Suchánek e7813fbafb Introduce EmergencyWatcher service
Change-Id: I45477ca84a99f620d182ef95e5627d421d38f077
2020-11-18 14:20:18 +00:00
Daimona Eaytoy ae29451ab8 Introduce a FilterCompare service
The scope is still quite limited, but as noted in a todo, we might want
to make this completely independent from the database, and add the use
case of ViewDiff.

Change-Id: Ie980fff0983b3e86037265e85da04444c809a6e8
2020-11-18 11:52:44 +00:00
Daimona Eaytoy 1bcfdc3b13 Introduce a FilterValidator
This moves a lot of things away from the AbuseFilter class. There's a
nasty static dependency on ChangeTags, but it's very limited anyway, and
it's going to be fixed once T245964 is resolved.

Change-Id: Ia7df4b4d3289c2722323f59ceecf3fdd38277785
2020-11-18 01:41:31 +00:00
Daimona Eaytoy 725ec052ed Add a FilterLookup service
Some pieces of code were updated to use Filter objects, while other
places are still to be updated. We also need to change the history part
to exclude actions somehow, cleanup the ViewEdit, reduce direct DB
access or anything mentioning DB fields outside of FilterLookup, etc.

Change-Id: I42b7ded685db76eddd45e4b1336f9828cba811ce
2020-11-18 01:17:47 +00:00
Daimona Eaytoy bad5a9a29c Make AbuseFilterViewEdit work with Filter objects
This requires adjusting some methods to work with Filter objects. Some
methods and tests are left in an inconsistent/suboptimal state, plus some todos
were added, but all of this is going to be remediated in another commit.

Change-Id: Id063ee73d97c7aef56323e1457d99704f77ab943
2020-11-18 00:52:37 +00:00
Daimona Eaytoy 71a61c2089 Add value objects to represent filters
This is just a start; next step is adding a factory/store method to
get/store these objects. And then use these value objects whenever
applicable.

Note: the actions-related code is still not fully implemented. This is
going to happen as part of the FilterLookup.

Change-Id: I5f33227887c035e301313bbe24d1c1fefb75bc6a
2020-11-04 12:56:14 +01:00
Daimona Eaytoy 1f8df50cb3 Add a service to retrieve the central DB
This is a thin wrapper around LBFactory and the global variable, that
can be injected in classes requiring it (no real class right now, but
that's going to change soon).

Also, remove some DWIM-style returns which made the code harder to
understand.

Change-Id: I1d28ad4a67f914103f3a17cda5f61b28070c7f1c
2020-10-31 12:32:46 +00:00
jenkins-bot ec5b9bef44 Merge "Add a service to retrieve the filter user" 2020-10-29 09:52:56 +00:00
Matěj Suchánek 77f6ecce13 Cleanup FilterProfiler API
Make FilterProfiler::getFilterProfile return stats unchanged,
in a structured way. Move computations to AbuseFilterViewEdit,
as they are only useful there. Don't return false on cache
misses, return arrays with zero values instead.

Bug: T266531
Change-Id: I8718cc31a5004340bf742315c7075e10a61fcbfd
2020-10-28 12:48:30 +00:00
Daimona Eaytoy 916234598d Simplify ViewEdit, last round
This deals with data inconsistencies in buildFilterEditor. Every
property of $row was tested in all 5 scenarios (also using Selenium) to
check when it's set. The result is in the normalizeRow method, which
aims to remove any inconsistencies, so that buildFilterEditor always
receives a "complete" row with all defaults set.

The code in buildFilterEditor is now cleaner (because there are no
isset() checks), and it gives us a unique place where we can set
defaults (rather than partly doing that in
loadRequest/loadFilterData/loadImport, and partly relying on isset).

This will be especially useful when introducing value objects to
represent filters, because now you just have to look at normalizeRow()
to tell which properties are allowed to be missing, and thus what "kind"
of filter object you need (see
I5f33227887c035e301313bbe24d1c1fefb75bc6a).

Additionally, reduce the properties that get passed around during
export/import, and make the selenium test try a roundtrip, rather than
relying on hardcoded data that may get outdated. A future patch will
refactor the import/export code.

Change-Id: Id52c466baaf6da18e2981f27a81ffdad3a509e78
2020-10-26 13:07:29 +00:00
Daimona Eaytoy cbea88f818 Add a service to retrieve the filter user
Unfortunately, this isn't using DI completely, because of the
User::newSystemUser call. I'm not even sure if we really need to call it
or we can just stick to new UserIdentityValue, but leaving like this for
now.
Also, the types were weakened to UserIdentity, so the transition is
going to be easy anyway.

Change-Id: I08f8fae0fcc622ff0ac3f86771476d06d1c18549
2020-10-26 14:06:53 +01:00
jenkins-bot 50ae561641 Merge "Simplify ViewEdit, round 2" 2020-10-25 09:10:11 +00:00
Daimona Eaytoy 18ade98339 tests: Move any profiling-related test to FilterProfilerTest
This commit removes several tests from AbuseFilterConsequences, thus
speeding it up a lot (especially because these tests were very slow,
with each test *case* taking up to 30s in the coverage job).

Everything is now covered by the new AbuseFilterFilterProfilerTest
which, although not being a pure unit test, is much much faster than
*Consequences.

Change-Id: Ic6b16d23ec99abee287f36093b8573505f9c613a
2020-10-24 18:09:26 +02:00
Matěj Suchánek 6d81fca76b Improve FilterProfiler coverage
Also improve documentation of some FilterProfiler methods.

Change-Id: I08198c643a7d2dac10e928914e8a5c7413f2543d
2020-10-24 16:23:47 +02:00
Matěj Suchánek 1445d5962a Introduce BlockAutopromoteStore service
This service is responsible for the blockautopromote feature:
(un)block autopromotion and check status.

The patch mostly moves code from static methods to the new class
and relaxes type hints (e.g. from User to UserIdentity).

Change-Id: I79a72377881cf06717931cd09af12f3b8e5f3e3f
2020-10-24 12:31:44 +00:00
Daimona Eaytoy 416dcd9ba3 Simplify ViewEdit, round 2
- Add a helper method to output an unrecoverable error, comprising a
   button to go back to the filters list;
- Move the token check to attemptSave, so to make the conditionals
  easier to read, and group errors together
- Make buildFilterEditor take an HTML parameter for the error, so the
  caller can specify whether it's error or warning
- Move the check for non-existing filters out of buildFilterEditor
- Add a bunch of typehints
- Don't set af_throttled and af_hit_count in the empty row template, but
  set af_deleted (these are only used in buildFilterEditor)
- Make AbuseFilter::translateFromHistory consistently include the af_global
  property (previously it would only be set for global filters; this error
  was introduced when first implementing global filters)
- The only user-facing change is that, when trying to use a custom
  warning/disallow message on a global filter, this is now considered a
  non-fatal error, so we now show the editing interface (and not just an
  unrecoverable error).

The next step is resolving the @todo in buildFilterEditor about null
checks.

Change-Id: I9d217dcac3f4cc0b26e53eca735cc327d5efc76d
2020-10-23 13:00:43 +00:00
Daimona Eaytoy 4c06dd52c8 Replace $wgAbuseFilterRestrictions with more specific variables
So that sysadmins can further customize the extension. It was also wrong
to use the same variable for many different things.

Note that there's no associated patch in wmf-config because we use the
defaults. However, before merging this patch, please recheck that
AbuseFilterRestrictions and AbuseFilterDisallowGlobalLocalBlocks aren't
used there (https://codesearch.wmflabs.org/operations/?q=AbuseFilterDisallowGlobalLocalBlocks%7CAbuseFilterRestrictions&i=nope&files=&repos=)

Bug: T175221
Change-Id: I7581b3ee6d9d11a6cf1599b8ff874e8c3d54adf4
2020-10-22 13:38:59 +00:00
Matěj Suchánek 93556284a0 Inject ChangeTagsManager to ChangeTagger
We decided to have the tag name provided by ChangeTagsManager,
so make ChangeTagger depend on it.

Change-Id: If3cbfd992f45651f47477031befffc0fd30f4a28
2020-10-21 16:30:43 +02:00
Daimona Eaytoy 9bc885b6b3 Add a ChangeTagger class
The logic about action IDs and the persistent buffer is now encapsulated
inside a single service, which is a step towards getting rid of global
state in the AbuseFilter class, and reducing the responsibilities of the
Runner.

An important change made here is that we now require a LinkTarget rather
than a Title. This removes a dependency on the Title class (a monster
object), makes tests simpler, and denies the need to inject a
TitleFactory. This means living without some bits of context (e.g. we're
no longer using makeTitleSafe to ensure a valid title, and we have to
build a "prefixedtext" manually), but this shouldn't be a problem, given
that the titles are only used to create a cache key: invalid titles are
not a problem, and concatenating namespace + title should always be
sufficient.

Bug: T265370
Change-Id: Iff59cd3d889454a482a89c16691bfefcc5ec0a12
2020-10-21 13:19:30 +02:00
Matěj Suchánek d91ddd2169 Cleanup filter id handling on Special:AbuseFilter/history
In particular, the interface shouldn't generate links to
"Special:AbuseFilter/history/0" (AbuseFilterHistoryPager::getTitle,
can be seen when visiting "Special:AbuseFilter/history").

Change-Id: Id3dc1bb4fc3c5e853603bf0ec04a6b1751f7d862
2020-10-10 11:40:46 +00:00
Daimona Eaytoy f0539e0c1e Represent new filters with null instead of 'new'
PHP is not strongly typed, so it's not a good idea to use scalars of
different types (here it's an integer vs the string 'new') to represent
different possibilities. This can have bad effects when type juggling
occurs, and it's also harder to figure out what the type of the
parameter can be (because a numeric ID might have been passed as a
string). Using integer vs null avoids all of this, and also allows us to
use nullable typehints.

These changes were partly copied from
If981cb35bf19a8469aa6c43c907e107cf8c65bc2 and should help with the
migration to the Filter value objects.

Change-Id: I8837d46c3c33761fea53f67b530b721dc7bd49b0
2020-10-10 12:23:50 +02:00
jenkins-bot c0defc1055 Merge "Add a new FilterProfiler service" 2020-10-10 10:08:58 +00:00
Daimona Eaytoy 9f2906e34b Reduce dependencies of AbuseFilter::saveFilter
This patch removes the dependency of saveFilter on the ContextSource
kitchen sink. It also removes some unneded dependency, and adds
$originalRow/$originalActions as parameter, rather than hacky properties
in $newRow that are easy to forget. The related test can also be greatly
simplified.

This also introduces a behaviour change: checking $newRow instead of the Request allows us
to account for values normalization done in
AbuseFilterViewEdit::loadRequest, and to also work correctly for imports
(and generally speaking, it makes the method suitable for an
AbuseFilterEdit API module, too).

Next step is moving this method to a service. Some signatures,
indenting, name choices etc. are subpar, but this is just because these
methods are temporary anyway.

Bug: T213037
Change-Id: I235b928d7b9c2ef1c46ea0bf3e3ed212500b4161
2020-10-09 11:52:02 +00:00
jenkins-bot c6ab7b8e1f Merge "Use triple equals in abuse filter parser tests" 2020-10-06 17:42:29 +00:00
Huji Lee d07717dd0d Use triple equals in abuse filter parser tests
The only exception is mwexamples-comparisons.t which intentionally
includes examples with = and == to test the "weak" version of the
comparison operator.

Bug: T262063
Change-Id: I6f92aadc69489da481a606bfda89617b8efbb261
2020-10-06 12:29:47 -04:00
Daimona Eaytoy bc9898f1a1 Add a new FilterProfiler service
Change-Id: Ib66c42ac220731f4e1da9ee6cfb5290759dd6494
2020-10-04 22:00:57 +00:00
Daimona Eaytoy 6c8a29698b Add test traits for uploads and account creation
Ideally, this might live in MediaWikiIntegrationTestCase. For the
createaccount one, AuthManager should also provide a method to log the
creation, because currently we are forced to copypaste that code here.

 - Add the missing tests for 'upload' in RCVariableGenerator, and adjust
the existing ones (delete file afterwards, more tablesUsed, use the
right extension).

 - Exclude from the coverage report a couple of lines which should
theoretically be unreachable. Escalate logging to WARN level, where it's
more likely to be spotted.

 - Remove an unused method (RCVariableGenerator::newFromID). This denies
   the need to maintain and cover it. We also don't want this generator
   to act as a factory.

Overall, this change brings the coverage for RCVariableGenerator to 100%

Bug: T201193
Change-Id: I425c3d9f6800f74eb6e4eda483b90cfb3bbbcb51
2020-10-04 13:16:58 +00:00
Daimona Eaytoy 2e13d58c74 Add tests for retrieving RC variables
This was also long overdue. Also fix a bug that caused page creations to
not be shown when examining past edits (using rc_last_oldid doesn't work
for page creations).

Bug: T201193
Bug: T262903
Change-Id: I5f7a994add12332c950904146248c5de7c2beee5
2020-10-04 12:43:04 +00:00
jenkins-bot a9654c3ab3 Merge "Refactor AbuseFilterView instantiation" 2020-10-04 12:28:24 +00:00
Matěj Suchánek eb81b92c06 Refactor AbuseFilterView instantiation
- Make a separate method which determines the view
  to be shown from subpage syntax and test it.
- Reduce circular dependency between SpecialAbuseFilter
  and AbuseFilterView. Use params to transfer information
  to views.

Change-Id: Ib9442ea5f9990a5c48f9b9e04055aa22bf7e456e
2020-10-04 13:15:04 +02:00
Daimona Eaytoy 60519e1886 updateVarDumps: avoid using services in the constructor
This is I4df27f3d02432c201c04d9fa118f0129b0a79778 striking again. Fool
me once, shame on thee, fool me twice...

Change-Id: Icea025a2c81e3b413b7bd9ece52866aeaf42937d
2020-10-03 23:23:37 +00:00
Daimona Eaytoy 7bc5248ed7 Selenium tests: wait for clickable button
It's possible that we try clicking this button before it's ready. This
theory is hard to verify because I get no problem locally, but this
shouldn't hurt.

Also specialize input selectors for a couple elements that might not be
univocally determined within the page.

Change-Id: Ida65c3c5fd4d8b3b35ecbee7e99977c71c7c4b96
2020-10-01 00:51:35 +02:00
jenkins-bot fa412f4e7e Merge "Rewrite the VariableHolder code to translate deprecated variables" 2020-09-30 09:10:37 +00:00
Daimona Eaytoy 1bdf4e5351 Rewrite the VariableHolder code to translate deprecated variables
The current code was more of a subpar, temporary solution. However, we
need a stable solution in case more variables will be deprecated in the
future (T213006 fixes the problem for the past deprecation round). So,
instead of setting a hacky property, directly translate all variables
when loading the var dump. This is not only stable, but has a couple
micro-performance advantages:
 - Calling getDeprecatedVariables happens only once when loading the
   dump, and not every time a variable is accessed
 - No checks are needed when retrieving a variable,
   because names can always assumed to be new

Some simple benchmarks reveals a runtime reduction of 8-15% compared to
the old code (8% when it had varsVersion = 2, 15% for varsVersion = 1),
which comes at no cost together with increased readability and
stability. It ain't much, but it's honest work.

Change-Id: Ib32a92c4ad939790633aa63eb3ef8d4629488bea
2020-09-29 15:06:14 +00:00
Daimona Eaytoy 62adeb3ce5 Add a lot of selenium tests for the editing view
The editing view is currently full of tech debt, brittle and surprising
code and whatnot. It's basically a miracle if it works without problem,
and it'd be an even bigger miracle if you could change something there
without breaking anything.

For these reasons, and because that class must be refactored as part of
the upcoming overhaul, this patch adds a bunch of selenium tests to test
the main functionality of that page.

In particular, these tests cover all possible cases (each corresponding
to a data source) for which buildFilterEditor can be called, which FTR are:
1 - View the result of importing a filter
2 - Create a new filter
3 - Load the current version of an existing filter
4 - Load an old version of an existing filter
5 - Show the user input again if saving fails after one of the steps
  above

Having automated tests to cover these cases means that we don't have to
manually test all the scenarios manually each time the class is touched.

Bug: T201193
Change-Id: I408e0a132905416effe0d6d6dc0921991edd66bd
2020-09-29 14:22:53 +00:00
jenkins-bot 7a684c487c Merge "Move some misplaced AbuseFilterParser entry points" 2020-09-29 13:51:17 +00:00
Daimona Eaytoy 55ba083b13 Introduce a KeywordsManager service
This will decouple a bit the huge and chaotic tangle of AF classes. Some
boilerplate code for AbuseFilter services is also added with this patch.

Note that this requires injecting a KeywordsManager in
AbuseFilterVariableHolder, or unit tests would fail. This is still
incomplete, and the Manager is only injected in tests, because
VariableHolder still has to be refactored.

The test for the UpdateVarDumps script had to be updated, because
serializing VHs in there was a bad choice. As pointed out in a comment,
the test is likely going to break again once we remove the BC code, but
I hope that we'll be able to remove the test at that point.

Change-Id: I12a656a310adb8c5f75cab63f6db9e121e109717
2020-09-28 23:03:52 +00:00
Daimona Eaytoy a1626a0d7f Move some misplaced AbuseFilterParser entry points
These methods had no reals reason to be static and belong to the
AbuseFilter class. Most of them were moved to Parser class as common
variations of the existing entry points. One was specific to the
EvalExpression API module and was moved there.

This change comes at no cost, and will make it possible to inject a
parser where needed.

Change-Id: Ifd169cfc99df8a5eb4ca94ac330f301ca28a2442
2020-09-29 00:36:08 +02:00
Daimona Eaytoy 8fa9e6625a Add tests for 'upload' action
This adds some coverage for the *VariableGenerator classes. It's still
not perfect, but something to start with in sight of future
refactorings.

Bug: T201193
Change-Id: Iafa85fb8623ea278ce6e42118df72751806382c2
2020-09-28 11:53:53 +00:00
Daimona Eaytoy 241a5db604 tests: Create dedicated classes for VariableGenerators
Also fix the test for _first_contributor vars to cover all variables,
use builtin methods to compute the var (rather than calling
setLazyLoadVar), and to be an integration test.

Change-Id: I2594439acc786e31bce1cd4373d3cbf434204eda
2020-09-28 11:53:47 +00:00
Reedy 41a403ebd8 Give AbuseFilterSaveTest::testSaveFilter() a return value for selectRow
Change-Id: Ifdf6d2155ec3d51600bceaa63832bb55a71599d3
2020-09-24 18:25:26 +01:00
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
jenkins-bot 92c3c8dbe1 Merge "Revert "Fix a test which will be broken by Hooks::run() migration"" 2020-09-16 08:10:07 +00: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
proc a31f4e46af
Strict type comparison
Bug: T248806
Change-Id: I039ab7f103bb37052987b815412b71f70643a6d2
2020-06-27 15:55:57 +01:00
jaredblumer 12f9be5e69 eslint: Update to eslint-config-wikimedia 0.16.0
* Update ESLint config with Selenium WebdriverIO test suite
* Update modules and Selenium pageobjects and specs per ESLint
requirements
* Update grunt-eslint package to 23.0.0 as required by
eslint-config-wikimedia 0.16.0

Bug: T254495
Change-Id: Ibfcf9115adedf9f2c3e7dac1ac626b41fc97b7c4
2020-06-08 21:17:50 -04:00
vidhi-mody 76025137a3 Selenium: Update to WebdriverIO v6
Update NPM packages: @wdio/* and webdriverio.

Bug: T253167
Change-Id: I11edb5dca8d1d45ffc336c805a40c502b815ce1f
2020-05-28 19:56:43 +05:30
vidhi-mody f2759a17c9 Selenium: Update to WebdriverIO v5
Update NPM packages: webdriverio, wdio-mediawiki.
Update ESlint configuration.

Replace NPM packages:
- wdio-mocha-framework with @wdio/mocha-framework.
- wdio-spec-reporter with @wdio/spec-reporter.

New NPM packages: @wdio/cli, @wdio/local-runner, @wdio/sync.

Replace:
- `browser.element` with `$`.
- `chromeOptions` with `'goog:chromeOptions'`.
- `password` with `mwPwd`.
- `username` with `mwUser`.

Bug: T253167
Change-Id: Ia26da6d0f56398a908bd4e99ae77db603647f533
2020-05-21 12:36:32 +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
Ed Sanders 2af7bc67c7 eslint: Remove unused rules
Change-Id: I306c25c3f604893dea24b542619832a5cd3d1759
2020-04-24 22:21:15 +01: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
Daimona Eaytoy 4a24c174f2 Fix a test for IPUtils 2.0.0
0.0.0.0/0 is not a valid range.

Bug: T218074
Change-Id: I562fefe66c0b692a99f546ca2d4c833dd508272a
2020-03-21 16:49:44 +00: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 391bbee53c Merge "Fix some edge cases in ViewEdit" 2020-02-08 16:36:19 +00:00
jenkins-bot c0b58d7699 Merge "Factor out variables-related methods" 2020-02-08 14:42:13 +00:00
Daimona Eaytoy 0834f37e42 Fix some edge cases in ViewEdit
Follow-up Iabd0ae5b18571f8cad44ef2d86bcf2519e7f95ba.

This patch:
 - Moves some save-related code to a separate method
 - Reduces conditionals nesting
 - Fixes an edge case where the content of the form would be
 wiped in case the token didn't match.
 - Adds another (basic) selenium test
 - Standardizes return types
 - Moves data load outside of buildFilterEditor

Change-Id: I89444b59f04c495c9ab59244151c8ed5d38cf0fe
2020-02-08 15:35:46 +01: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 8fea62529b Merge "Fix AbuseFilterCachingParser violating return type constraint" 2019-12-27 10:04:57 +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 d5ab147dcf Fix AbuseFilterCachingParser violating return type constraint
This is identical to I8a3c31e7385283d95b4712d457784016239a0b3b, except
for the array append case.

Bug: T236870
Change-Id: Iac033ba467232f6ff110d575920e968759ce0e15
2019-12-04 18:27:46 +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
jenkins-bot 2d2e524dca Merge "Tokenizer: don't strip backslashes from \x" 2019-11-22 13:36:49 +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 c73381b6db Tokenizer: don't strip backslashes from \x
Bug: T238475
Change-Id: I8c2ea6ad369946df93440eece60d456dc1a3fd7a
2019-11-16 16:21:39 +01:00
Daimona Eaytoy 98bcad25c3 Also parse numbers with the new syntax and hard-deprecate the old one
This will allow people to switch their filters to the new syntax. The
deprecation warning is now more exhaustive, and the info() warning is
kept to ensure that everything proceeds smoothly.
The regex v2 has also been fixed to:
 - Consume all the digits/letters on the right (*)
 - Have named groups
 - Be created dynamically with other constants

(*) The previous version of v2 could complete the match and leave
digits/letters on the right when encountering numbers with the old
syntax, hence dropping support too early. We also cannot use a word
boundary (\b) because that would prevent matching numbers with trailing
dots (e.g. "5.").

Bug: T212730
Change-Id: Ibf6ac571f6b5c09149d69a19c38240ce6b024dff
2019-11-12 11:52:38 +00: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
jenkins-bot 91bc961712 Merge "Check for 0-like floats passed to the modulo operator" 2019-11-10 11:51:28 +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
jenkins-bot 7ff4b95aec Merge "Expand the list of types that can be cast to int" 2019-11-10 11:00:36 +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
Daimona Eaytoy a7b28369ea Expand the list of types that can be cast to int
Bug: T237624
Change-Id: I2220cb8a8ec998a433a4469d7e0591ec0b4f2b12
2019-11-07 15:14:17 +01:00
Petr Pchelko 915b9a1538 Remove usages of deprecated User methods
Bug: T220191
Change-Id: I54e20870a32ff98b41a98495694ff563c4c4c5ca
2019-10-30 12:51:01 +00:00
Daimona Eaytoy 3a9eac9ad5 Unbreak filter edit form
In Ib7427e15f673a575738489476e604c387f449ddd, I thought that $parameters could've only been null if $action wasn't
enabled, but actually, they're null even if the action is just not set.
Which is true for all actions when creating a new filter, and all
non-set actions when editing an existing one.

Hence, revert the part that touched ViewEdit.

Also add a selenium test to ensure that warn parameters are visible.

Bug: T236286
Change-Id: I8150baa077208eb1fc54ebc1d8415a243d0f3bd3
2019-10-23 18:50:44 +02:00
James D. Forrester 4d988471be build: Upgrade mediawiki-codesniffer to v28.0.0
Change-Id: I7ef6ec1614718c016562281a166867ee3bd93df7
2019-10-09 18:34:07 +00:00
Max Semenik b3d11b48cb tests: setExpectedException() is deprecated
Bug: T192167
Change-Id: I899a8f03c6cc1f79f58bec09c2d8b2ba10b895d8
2019-10-08 16:31:15 -07:00
Daimona Eaytoy 0646b4fe7d Add config for Selenium and basic tests
This is some sort of Hello World for selenium. This patch adds the
config files and a couple of very basic tests.

Bug: T214478
Change-Id: I8193b4edb40332bea1d08e24ec020bf36004320d
2019-09-17 16:23:07 +00:00
jenkins-bot 48713c824b Merge "Throw AFPUserVisibleExceptions for empty operands in CachingParser" 2019-09-15 08:36:39 +00:00
Daimona Eaytoy a4e25c1ac9 Throw AFPUserVisibleExceptions for empty operands in CachingParser
Instead of TypeErrors. Basically, only empty parenthesis had to be
fixed.

Bug: T156096
Change-Id: I019615c7bfaa179c2184b5d3ea2c6b5da91366e3
2019-09-14 18:35:40 +00:00
jenkins-bot 45d7bd5971 Merge "CachingParser: ensure to catch errors inside short-circuited blocks" 2019-09-14 01:56:35 +00:00
jenkins-bot b8ad85cac7 Merge "Annotate the AST with var names before caching the AST" 2019-09-14 01:03:53 +00:00
Daimona Eaytoy 6e9a9a3bc2 CachingParser: ensure to catch errors inside short-circuited blocks
This is similar to the old parser: when discarding a node, actually
evaluate it if short-circuit is not allowed.
Add a whole lot of tests for all possible exceptions.
Move the logic to extract a message from an AFPUserVisibleException away
from the parser, to keep unit tests working.

Bug: T232498
Change-Id: I31ee4e255c6a87dd693b9bcd582539fdf57acd45
2019-09-13 21:13:15 +00:00
Daimona Eaytoy 004ccfdb5c Annotate the AST with var names before caching the AST
This implements T230982#5475400, and it should speed up the CachingParser by roughly 40%.

Bug: T230982
Change-Id: I803cc58637d50eb90e57decf243f5ca78075d63d
2019-09-13 19:43:50 +00:00
jenkins-bot cfad7d6f14 Merge "Actually return errors for action=edit API" 2019-09-10 19:59:03 +00:00
Bartosz Dziewoński 82b6f191d4 Actually return errors for action=edit API
Setting 'apiHookResult' results in a "successful" response; if we want
to report an error, we need to use ApiMessage. We already were doing
this for action=upload. Now our action=edit API responses will be
consistent with MediaWiki and other extensions, and will be able to
take advantage of errorformat=html.

Since this breaks compatibility anyway, also remove some redundant
backwards-compatibility values from the output.

To avoid user interface regressions in VisualEditor, the changes
I3b9c4fef (in VE) and I106dbd3c (in MediaWiki) should be merged first.

Before:
    {
        "edit": {
            "code": "abusefilter-disallowed",
            "message": {
                "key": "abusefilter-disallowed",
                "params": [ ... ]
            },
            "abusefilter": { ... },
            "info": "Hit AbuseFilter: Test filter disallow",
            "warning": "This action has been automatically identified ...",
            "result": "Failure"
        }
    }

After:
    {
        "errors": [
            {
                "code": "abusefilter-disallowed",
                "data": {
                    "abusefilter": { ... },
                },
                "module": "edit",
                "*": "This action has been automatically identified ..."
            }
        ],
        "*": "See http://localhost:3080/w/api.php for API usage. ..."
    }

For comparison, a 'readonly' error:
    {
        "errors": [
            {
                "code": "readonly",
                "data": {
                    "readonlyreason": "foo bar"
                },
                "module": "main",
                "*": "The wiki is currently in read-only mode."
            }
        ],
        "*": "See http://localhost:3080/w/api.php for API usage. ..."
    }

Bug: T229539
Depends-On: I106dbd3cbdbf7082b1d1f1c1106ece6b19c22a86
Depends-On: I3b9c4fefc0869ef7999c21cef754434febd852ec
Change-Id: I5424de387cbbcc9c85026b8cfeaf01635eee34a0
2019-09-09 20:15:19 +02:00
Daimona Eaytoy 7b06be0204 Allow dangling commas in variargs
This is because there are many filters using this feature. Moreover, it
could make it a little easier to add new arguments, just like dangling
commas in PHP arrays do.
Also re-align the CachingParser code of doLevelFunctions to the one in
the old Parser.

Bug: T153251
Change-Id: Ie4325159f47310788da57415a5e36e62aa4efad0
2019-09-07 11:19:14 +02:00
jenkins-bot 5be19f6f65 Merge "Add a 'strict' option to VariableHolder::getVar" 2019-09-05 19:23:23 +00: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 13b1e880f2 Hotfix other DUNDEFINED casts to bool
These were spotted on testwiki with wmf.21.

Change-Id: Ic4d67a2b83aedfeb574fa1363a9fc618b2862f95
2019-09-04 18:06:22 +00:00
Daimona Eaytoy ce8539e2a5 Move parser tests back to /unit
Using `new LanguageEn()` involved a global, so use a MockObject instead.
Also fix LoggerFactory usage in Tokenizer to use DI instead.

Change-Id: I94d03f9459ab6444e239386eb96a0c2434bfe3dc
2019-09-03 13:23:11 +00:00
Kosta Harlan 984e06639d Move non-unit tests back into default (integration) directory
Follows-up Id8412e2b8a4e873fd4821ecc1a3c95710be9a870.

Change-Id: Ib92cfbb637e0143a5481212f11a6e511929d6801
2019-09-01 19:57:26 +00:00
Daimona Eaytoy f7812ea7a3 Remove redundant User::addToDatabase call in tests
We don't need to call it in the constructor, as long as the call in
setUp() is moved to before we start adding groups and checking blocks.

Follows-up Id8412e2b8a4e873fd4821ecc1a3c95710be9a.

Change-Id: I339363499f99295a83004074d6a44574cd622a58
2019-08-31 16:35:22 +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
Daimona Eaytoy 71730f7d44 Warn if a function has been given too many parameters
While this is not as important as throwing for too few parameters, IMHO
it's still important to fail in this case. Mostly because if a function
receives too many parameters, chances are that who wrote the filter
didn't do that intendedly, and thus there may be a hidden bug.
Bonus: fix a few docblocks.

Bug: T230803
Change-Id: Iac2931f17b50ace8c8f4c2faa44b3f54ca134c54
2019-08-26 20:29:49 +02:00
Aryeh Gregor 4c8dac4dc6 Change config only before we've started testing
In general, it's not safe to change configuration in the middle of a
test, because services could wind up in an inconsistent state. In
particular, I'm trying to have setMwGlobals() reset services, which will
cause stuff to break if it happens in the middle of a test. So just
specify the settings you want up front, like in setUp().

Change-Id: I00e35ecea6a27468674b2a6e7d9d9eb6518e3bd5
2019-08-26 14:26:44 +03:00
jenkins-bot ff2f6ee26f Merge "Add a new class for the CachingParser's AST" 2019-08-25 18:00:24 +00:00
Daimona Eaytoy d515af0ae6 Add a new class for the CachingParser's AST
This allows a little bit more of abstraction: we can store other data in the
tree, without having to store it in a specific node (e.g. the variables map,
which is still unused). It also adds a few typehints, and specializes
the return value of eval'ing the AST: previously, it was the one of
evalNode, which wasn't guaranteed to be an AFPData. Now we have this
guarantee. Last but not least, we can now measure runtime metrics for
evalTree, which doesn't recurse.
Bonus: fix a check in the old parser, which used the wrong variable when
reporting outofbounds errors.

Change-Id: Iff806793b1d968e9bb6220f1459f3d0ac587c7da
2019-08-25 17:29:16 +00:00
jenkins-bot 6196801178 Merge "Log more empty operands" 2019-08-24 20:53:01 +00:00
Daimona Eaytoy 2d031d0bee Log more empty operands
And fix a couple of minor bugs.

Bug: T156096
Depends-On: I3b85087677607573f4fa68681735dc35348dcd87
Change-Id: Ia4c713a1d45827f6a8bc5566a8d8835c49f8108a
2019-08-24 19:59:53 +00:00
jenkins-bot 47838715fa Merge "Allow if without else" 2019-08-20 20:12:19 +00:00
jenkins-bot 5e605aaa62 Merge "Even better handling of DUNDEFINED" 2019-08-20 20:00:52 +00:00
jenkins-bot bf8ccccade Merge "Fix a bug in the return value of the CachingParser" 2019-08-20 19:58:38 +00:00
Daimona Eaytoy af7744781f Allow if without else
Bug: T230727
Depends-On: I8e7f7710b8cb37ada8531b631456a3ce7b27ee45
Change-Id: I3b85087677607573f4fa68681735dc35348dcd87
2019-08-20 19:36:14 +00:00
Daimona Eaytoy 963221ad6d Even better handling of DUNDEFINED
Ensure that the variable isn't set before marking it as DUNDEFINED:
that's only for when we cannot use a default, but if the variable is set
we already have one. Most notably, this fixes conditionals handling: right
now, if you have a conditional with an assignment in both
branches, the variable will be undefined. That's obviously wrong, so
it's fixed in this patch.
Plus: catch only AFPExceptions in a test to avoid unintentionally
catching the assert exception; simplify some assignments using wfSetVar.

Depends-On: I446a307e5395ea8cc8ec5ca5d5390b074bea2f24
Change-Id: I8e7f7710b8cb37ada8531b631456a3ce7b27ee45
2019-08-20 19:17:30 +00:00
Daimona Eaytoy fa76405ea7 Fix a bug in the return value of the CachingParser
This has always been wrong, and remained unnoticed. Also added a
typehint for added safety.

Change-Id: I8a3c31e7385283d95b4712d457784016239a0b3b
2019-08-20 20:54:19 +02:00
Daimona Eaytoy aa867bd370 Better handling of function params in CachingParser
This patch includes various fixes to how func arguments are handled in
CachingParser:
- Add a comment about a future improvement of checkSyntax, which we
  could limit to try building the AST.
- Having enough args for each function is now also checked when
  building the AST. This allows implementing the previous point without
  stopping to report notenoughargs at syntaxcheck-time (otherwise it'd be
  a runtime error). And it also ensure that we check for the params count
  inside skipped branches, e.g. inside if/else: these were already only
  discovered at runtime in CachingParser. The old parser is not affected
  by this change, because when checking syntax it will always execute
  all branches, and at runtime it will skip braces altogether.
- Fix arg count for CachingParser, which previously added a bogus param
  in case of a function called without parameters. This was fixed for
  the other parser in I484fe2994292970276150d2e417801453339e540, and I
  just ported the updated fix. Also note that the CachingParser was
  already failing for e.g. `count()`, but instead of complaining about
  missing arguments, it failed hard when trying to pass NULL to
  evalNode.
- Fixed some tests not to use setExpectedException, which caused the
  previous point to remain unnoticed: calling that method prevents the
  loop from continuing, and thus only the AbuseFilterParser part was
  being executed. The new implementation checks the exception ID and is
  thus more future-proof if the i18n message changes.
- Fixed some function names in error reporting for the old parser.
- The arg count is now checked outside of the function handlers, thus
  it's no more necessary to call checkEnoughArguments at the beginning
  of each handler. This also produces clearer error messages in case of
  aliases (e.g. set/set_var).
- Check the args count even if some of the args are DUNDEFINED. This is
  much easier now that the check is outside of the handler. This will
  make syntax check fail for e.g. `contains_any(added_lines)`.

Bug: T156095
Change-Id: I446a307e5395ea8cc8ec5ca5d5390b074bea2f24
2019-08-20 15:32:02 +00:00
jenkins-bot 1f45336157 Merge "Move keywords handlers to the Parser" 2019-08-20 14:16:10 +00:00
jenkins-bot f18d0814e2 Merge "Make several AFPData functions non-static" 2019-08-20 14:06:02 +00:00
Daimona Eaytoy 430ba818d0 Add test for multiple conditions inside conditionals
The regression itself was fixed in
I980aec3481a52ecc35f1811a366014a5581a7cdb, so this patch only adds a
test for it.
Also remove a comment about CachingParser failures: we don't want to
encourage people to remove it from tests anymore.

Bug: T152281
Change-Id: I3ad49050ea49bf45d3226878e091da3c8dbefdb1
2019-08-12 18:18:05 +02:00
Daimona Eaytoy 3f171dc0a5 Move keywords handlers to the Parser
Just like we do for functions, it doesn't really make sense to have
keywords separately, in AFPData.

Change-Id: I208a9b1ce2bd12038e9fbcc515c48d604ec80eb8
2019-08-12 14:29:56 +02:00
Daimona Eaytoy 2fdf091eb9 Make several AFPData functions non-static
The keywords-related ones will be handled in a subsequent patch.

Change-Id: Ifcfad438023ef136dc6f2cd5529e867df9b23789
2019-08-12 14:12:16 +02:00
Daimona Eaytoy 69ad23da98 Ban variable variables
As explained on phab, it's not worth the effort of keeping this feature.

Bug: T229947
Change-Id: Ic6067cab8e1ede98545e704888c99e2ed9a004e4
2019-08-11 01:47:35 +00:00
jenkins-bot 8ee442234f Merge "Move "block-autopromote" key from $wgMainStash to 'db-replicated'" 2019-08-07 23:07:45 +00:00
jenkins-bot 1fa5eef94c Merge "Overhaul Blockautopromote action" 2019-08-07 23:03:08 +00: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
jenkins-bot 5a067f7237 Merge "Add tests for empty operand logging" 2019-08-06 17:38:31 +00:00
Daimona Eaytoy b91db1d7be Add tests for empty operand logging
Follow-up 5f4491f9aa.

Change-Id: I80ca8c3c75f7de23cf9ab16aa66a240e9981c395
2019-08-06 17:17:27 +00:00
Daimona Eaytoy 2ed6272bb2 Partly handle set and set_var in shortcircuit
This is more complicated than the := operator, because the var name
could be a complicated expression, and we have to handle a function
call. This patch only covers the case where the variable name is a
literal, which is enough for WMF production.

Bug: T214674
Change-Id: I6c0f8e95663919a0235b5ccf0c88ad0a539315a7
2019-08-06 16:14:34 +02: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
jenkins-bot 19182606c1 Merge "Merge global profiling keys" 2019-08-04 18:40: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 517919fca8 Allow accessing offsets of built-in variables
I5ec4ab44c4e88aaf18c0d7b73355d27050beeda7 almost fixed this bug, but we
also have to make it possible to access builtin variables as arrays.
This will only make sense for a few variables (e.g. added_lines and
removed_lines), but I don't think we should validate it when checking
syntax.

Bug: T198531
Change-Id: I417e1b8d4802bbfccd091ce5c7617659cfd1e4ea
2019-08-04 17:14:44 +00:00
jenkins-bot c0b6267022 Merge "Use milliseconds for time profiling" 2019-08-04 16:12:59 +00:00
jenkins-bot f7fd6a6daf Merge "Move per-filter matches profiling to per-filter data" 2019-08-04 16:07:58 +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 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 a85e1ccc59 Make AbuseFilterParser::$funcCache non-static
Change-Id: I312efe3ce4d1f06e697aa4564aeec1bacbaf97d3
2019-08-03 09:19:49 +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
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 9937f8b050 Remove extra file from parser tests
Added in I5a14d4b2bc3ffd9caaaa095f16f36b9b6009db05, but .r files aren't
used anymore since I6c06e596587750c4ebaabafbd277bc75eeb436a5, and I
forgot to remove the file upon rebasing.

Change-Id: Id688d215b1136bd0a04b8c0d8d8d16de5da1295e
2019-07-15 12:22:09 +02: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
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
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 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
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 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
Daimona Eaytoy e86d4bc124 Simplify code for stashedEdits tests
Using the new PageEditStash class allows to simplify a bit the
integration tests for edit stashing. As I wrote in a ToDo, it may be
enough to manually run the hook, but that's left to do as a follow-up.

Change-Id: I3389a6961b4f39ecd980be2f429c23f8b7706a15
2019-06-24 11:13:59 +02: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
petarpetkovic c02590f555 Fix "succesful" typo
Change-Id: Ibd92f6de8b03098e7bdc8c4fc5e3f6cfaba95bdf
2019-06-14 03:08:41 +03: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
jenkins-bot 369ce36be7 Merge "Tokenizer caching back to APC" 2019-05-28 19:10:22 +00: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
jenkins-bot 112787020d Merge "Support for PermissionManager changes at https://gerrit.wikimedia.org/r/c/mediawiki/core/+/502484" 2019-05-28 11:24:53 +00:00
Vedmaka f293b3b7be Support for PermissionManager changes at
https://gerrit.wikimedia.org/r/c/mediawiki/core/+/502484

Change-Id: I258f02e286b6ba0387e1bff540a744fafb03dc55
2019-05-28 09:33:28 +00:00
Daimona Eaytoy 4b5c7c0198 Add missing covers/group tags
Including CachingParser as follow-up of
I980aec3481a52ecc35f1811a366014a5581a7cdb.

Bug: T201193
Change-Id: I9905efb2b2e61b330c42275c9ccfab2f24750bd4
2019-05-25 14:10:07 +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 b3707106e9 Reset MWTimestamp in tearDown
Follow-up of I5ee7ba44a6cd82a5ddb24fb4127af04d96e647f4.

Change-Id: Icf288d7c4a9d087e7e1cd8a6e8c8cc9dac20e532
2019-05-24 16:54:29 +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 00b9791349 Add unit tests for stashed edits
This is an important part to cover, and should be further expanded.
Also, fix a couple of minor things around, including making some methods
non-static.

Bug: T201193
Depends-On: I5e35d773904a62105767ce6d7d962ab5525c2d12
Change-Id: Ib17821240b25c972a187e6b5eae42c5ada6c65e7
2019-05-23 08:47:25 +00:00
jenkins-bot c52850aae7 Merge "Add missing limits to explode() calls" 2019-05-15 15:06:18 +00:00
Thiemo Kreuz c6f20a64dd Add missing limits to explode() calls
This is fixing potential bugs where invalid strings with more than one
comma have silently been accepted.

Change-Id: Ib1e7d0c99973f243ef6faad6389bab688187c1cf
2019-05-15 16:14:12 +02:00
Thiemo Kreuz fa3ce90851 Remove comments literally repeating what the code says
I find it obvious that a file called "AbuseFilterTokenizerTest" is a
"test for the AbuseFilterTokenizer class". A comment that is just
repeating this information is typicalls not helpful, but distracting
and a potential source of mistakes, e.g. when stuff is copy-pasted,
but the comment not adjusted.

Change-Id: I1d4cc06e9e5631955ff73bf675090cf9c33c9390
2019-05-15 16:04:32 +02:00
Thalia f23905c402 Remove call to deprecated User::isBlocked
Change-Id: Ibb7412f8aa08a745a211b9b0581ccb6b0ca9eff5
2019-05-14 13:14:57 +01:00
Daimona Eaytoy 2276d8ed2a Refactor tokenizer caching
Split a method, use WAN cache so that we're enabled to use
getWithSetCallback, pass the "version" option there and adapt the test
to it.
Follow-up of I9b3bc36b552901bc6ca7609ee51e80be2979a9c4

Change-Id: I4dd81a723e2bdb828b90594ad66a3918d8ec5b6c
2019-04-23 19:38:10 +02:00
jenkins-bot 968bd9b817 Merge "Add tests for tokenizer caching" 2019-04-17 23:27:19 +00:00
Aryeh Gregor b222330a61 Don't try to move onto an existing page in tests
I didn't fix every case where this happens, just what blocks
I6ddcc9f34a48f997ae39b79cd2df40dd2cc10197 from landing.

Change-Id: I971e619eb76c4474fe037fad258f9c496717bf41
2019-04-17 17:23:23 +03:00
Daimona Eaytoy 4b10a544ab Add tests for tokenizer caching
Caching the result of the tokenization is pretty important
performance-wise, so this test ensures that caching works as expected.
I have also extracted the method used to generate the cache key for
easier testing, and moved the cache instance to a class member because
otherwise that piece of code can't be tested...

Bug: T201193
Change-Id: I9b3bc36b552901bc6ca7609ee51e80be2979a9c4
2019-04-15 16:59:55 +02:00
Daimona Eaytoy ec110c657b Add tests for various data type casts
These are the ones which other tests don't cover, mostly because no
filter syntax can trigger those cases. This patch should bring coverage
for AFPData to 100%.

Bug: T201193
Change-Id: I997576141943959d4602a9f839311108928ec766
2019-04-14 14:08:57 +02:00
Daimona Eaytoy 909eec6716 Tweak coverage part 2
Follow-up of Ic30883f7d261d974a2be46308d023e2714119e95, with two files
that I forgot to git-add and a repositioning of comments to avoid the
last bracket to be reported as uncovered.

Bug: T201193
Change-Id: I6bf7e5892a0f49f6a138792f0aedf230a70c18a8
2019-04-13 19:26:01 +02:00
Daimona Eaytoy 4bcb64b01a Increase code coverage a bit
This patch mostly adds coverageIgnore comments for intendedly
unreachable code etc. Some of them could be made testable by adding a new
filter function (e.g. array cast), but this patch is meant to be
comment-only (aside from the parser test).
Ignoring coverage for these lines makes some methods reach 100%
coverage, which in turn makes it easier to look at the coverage chart
and identify at a glance which parts of the code *really* need to be
covered.

Bug: T201193
Change-Id: Ic30883f7d261d974a2be46308d023e2714119e95
2019-04-13 18:30:14 +02:00
Daimona Eaytoy 8293ec176f Add tests for storing and loading the variables dump
These are specific tests for storeVarDump and loadVarDump, both alone
and in the context of running filters.
Also, include disabled variables in the VariableHolder object if they're
saved in the DB.

Bug: T201193
Depends-On: Ia5c477edc8733bb1994cb6d01e1371ed496c8bcb
Change-Id: I5e35d773904a62105767ce6d7d962ab5525c2d12
2019-04-12 08:03:33 +00:00
jenkins-bot c0da9ff3ac Merge "Clean AbuseFilterParserTests" 2019-04-11 21:46:50 +00:00
Brad Jorsch b59f19d675 AbuseFilterTest: Don't use $wgUser when creating pages
Which means we have to pass a user to WikiPage::doEditContent().

Follows up Ifbcd9adf3.

Change-Id: I1bd0288cc132627d75b4001219522ec5e952eda7
2019-04-09 12:25:34 -04:00
jenkins-bot cc670f0a07 Merge "Clean the AbuseFilterTest class" 2019-04-06 14:47:52 +00:00
jenkins-bot efe32b7c93 Merge "Add doc for every class member" 2019-04-06 14:37:19 +00:00
jenkins-bot d53c84da36 Merge "Restore check for dividebyzero" 2019-04-06 12:35:23 +00:00
jenkins-bot e03488b66a Merge "Overhaul tag selector" 2019-04-06 12:35:20 +00:00
Brad Jorsch 5ace1121b0 Actually create user in AbuseFilterConsequencesTest
If the User passed to $logEntry->setPerformer() represents a creatable
username, then it has to actually exist so the actor row can be created.

Bug: T188327
Change-Id: Iab2fc9593a020ffacd219d644103d685028e3336
2019-04-05 12:35:25 -04:00
Daimona Eaytoy 0ff581e246 Clean AbuseFilterParserTests
Mostly delete result files and assume the result is always true. The few
exceptions were either moved to standalone test, or inverted.

Change-Id: I6c06e596587750c4ebaabafbd277bc75eeb436a5
2019-03-23 12:59:03 +01: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 9144f20245 Restore check for dividebyzero
Follow-up of I1721a3ba532d481e3ecf35f51099c1438b6b73b2. This is the only
wrong replacement: strict checking will let 5 / 0.0 pass, with
unexpected results. Adding a regression test for it, too.

Change-Id: I25dbe9fafa92fd9a11bd8bc6ab8e66f305b8d48e
2019-03-23 11:38:39 +01: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 d6c649bb0d Overhaul tag selector
If "tag" option is selected and the form is submitted without adding any
tag, just show it blank instead of adding an empty tag to the topbar.
Separately validate the empty tag case (and added a test for it).

Bug: T203353
Depends-On: I3b2e763bd8835207dc5df1db43d3e1881e6961c3
Change-Id: I8884b739fd17fa2eace5aac8775d3524aa606f1f
2019-03-17 14:04:50 +00:00
Daimona Eaytoy bedbe36744 Add doc for every class member
Adding PHPdocs to every class members, in every file. This patch only
touches comments, and moved properties on their own lines. Note that
some of these properties would need to be moved, somehow changed, or
just removed (either because they're old, unused leftovers, or just
because we can move them to local scope), but I wanted to keep this
patch doc-only.

Change-Id: I9fe701445bea8f09d82783789ff1ec537ac6704b
2019-03-17 11:40:24 +01:00
jenkins-bot 3f3e98fbc5 Merge "Fix shortcircuit for consecutive operations" 2019-03-17 10:04:14 +00:00
Daimona Eaytoy 683e94cdd3 Clean the AbuseFilterTest class
Remove all globals, make methods non-static, improve assertions and
computing some variables, add names to the tests and other minor
improvements.

Change-Id: Ifbcd9adf34d173d0da0aa568fc6f91fdc2d61609
2019-03-17 11:04:10 +01:00
jenkins-bot e2f1880922 Merge "Don't use wgLang and wgContLang" 2019-03-17 09:53:16 +00:00
jenkins-bot 65a4c26804 Merge "Remove exclusions for Generic.Files.LineLength" 2019-03-17 09:49:38 +00:00
Kunal Mehta 577f4dab93 Migrate to new phan
Bug: T216904
Change-Id: I30864bd3d7f9b9ab674bf6589cd9e5e3aed5bb8d
2019-03-16 09:41:23 +00:00
Daimona Eaytoy dd4b579695 Remove exclusions for Generic.Files.LineLength
Keep it only for filters definitions in ConsequencesTests.

Change-Id: I305c7f496a29b20a3ee1d34479d1e4cb9252060a
2019-02-23 10:12:07 +01:00
Thalia 540a557a59 Replace calls to deprecated Block::prevents
Where prevents is used as a setter, use the new setter methods;
where it is used to determine whether a block blocks the target
from editing their talk page, use appliesToUsertalk.

Block::prevents was deprecated and replaced by several other
methods in I0e131696419211.

Bug: T211578
Change-Id: I166cc6f64c0f895ff8c631d2655c1c3208131371
2019-02-22 19:29:02 +00:00
Thiemo Kreuz 3993a7ea15 Replace @expectedException with $this->expectException()
The @expectedException annotation got deprecated in PHPUnit 7.5, and
removed in PHPUnit 8.0. This was done because the annotation does have
two disadvantages:
* The class name is encoded in string, where it is not easy to find for
  all IDEs and tools.
* it did not allow to say exactly *when* the exception is expected.

Change-Id: I85f0b5f44b2f400a121115d402b64827ea534c32
2019-02-19 10:58:16 +01:00
Daimona Eaytoy 6f4bfc9597 Fix shortcircuit for consecutive operations
Using break could halt parsing between operations, instead use continue
to parse all operations.

Bug: T214642
Change-Id: If67ddaffef280c2448c55ae536013758617bba68
2019-02-08 17:55:59 +00:00
Tim Starling c889c2990c In tests that create users, add 'user' to $this->tablesUsed
Change-Id: I7d2c6b304974d487e1b7727f594d0843ff080a7d
2019-02-08 16:40:17 +11:00
Daimona Eaytoy 51120e51c5 Don't use wgLang and wgContLang
For wgLang, there's a Language object available in the proximity, so just pass it.
For wgContLang, use MediaWikiServices.

Change-Id: Ic492007f2d5eeb8048d0919a4b9b7dd98c15c350
2019-02-06 12:00:44 +01:00
jenkins-bot 15a8340ee1 Merge "Reject empty warning and disallow messages when validating a filter" 2019-01-31 21:28:17 +00:00
Daimona Eaytoy 0f041e8282 Split AbuseFilterConsequencesTest tests in several methods
This makes the code easier to maintain and more flexible, plus adds
several tests. Some flaky tests are also improved.

Depends-On: I57ce67c5202c8574fcf1957999a6999fec264cb7
Change-Id: Ibb5322bca93b464e9014b53644c04f2bc1141e72
2019-01-23 21:26:25 +00:00
Daimona Eaytoy 26b783f062 Use data provider's array keys to specify test description
We just passed the description as a parameter, but it's much quicker to
use it as the key in the data provider: PHPUnit will automatically
display it in case of failure, so that we don't have to do that
manually (and still get messages like "failed with data set #7").

Depends-On: I8edcca17ecdcf71397cc9b0d101e8b13ac112047
Change-Id: I57ce67c5202c8574fcf1957999a6999fec264cb7
2019-01-23 21:26:17 +00:00
Daimona Eaytoy 0e6b783ed4 Reject empty warning and disallow messages when validating a filter
Right now, we allow empty messages, and when the "warn" action is
executed we use "abusefilter-warning" if no message is specified.
However, this also produces a PHP notice while editing a filter with
empty message (see Phab). With this patch, empty messages will be
rejected, and a follow-up will be discussed on Phab.

Update: added disallow message as follow-up of
Ic1de03a6944c43a346fa317ee0a217551f0d284a.

Bug: T203353
Depends-On: I8df247f61d9f3769e9580544f324dd174811e939
Change-Id: I71b1f81d10c02de4de141b1ab9b630d05cf4619c
2019-01-21 14:06:54 +01:00
jenkins-bot df2da23d29 Merge "Add unit tests for custom disallow messages" 2019-01-19 12:21:02 +00:00
jenkins-bot b44984c50a Merge "Remove unused stuff" 2019-01-19 12:18:22 +00:00
jenkins-bot 575646393b Merge "Improve code readability" 2019-01-19 12:11:06 +00:00
jenkins-bot a2bee3bcf3 Merge "Simplify parser methods" 2019-01-19 12:11:04 +00:00
jenkins-bot 0d4e982069 Merge "Reduce code duplication" 2019-01-19 12:00:47 +00: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
Daimona Eaytoy f12fdb4a32 Add unit tests for custom disallow messages
Follow-up of Ic1de03a6944c43a346fa317ee0a217551f0d284a, adding some unit
tests for this newly introduced feature, plus a couple of tweaks for
both tests themselves and i18n.

Change-Id: I8df247f61d9f3769e9580544f324dd174811e939
2019-01-05 10:58:47 +00:00
Thiemo Kreuz 8ccb9839e5 Add test to guarantee tag uniqueness
This is a direct follow up for the bug fixed in Iebbdeac.

Change-Id: I5cc5618aa6161460534804e46a8a3568d1af9af3
2018-12-31 18:26:47 +01:00
daniel 688eccea47 Expose text from all slots to AbuseFilter
This is a first step towards MCR support in AbuseFilter. The textual
representation of all slots is concatenated. Since AbuseFilter uses
getTextForSearchIndex to determine the textual representation of
content, blind concatenation should not break any assumptions
made by AbsueFilter rules: this naive approach is no worse than
AbuseFilters handling of non-textual content in general, and should
work fine for textual content.

Bug: T209291
Change-Id: Ic141085cad2e11bfe106fe83dafcb35ac31206ba
2018-12-05 09:24:08 -08: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
Amir Sarabadani fd3e3e78cb Migrate AbuseFilterConsequencesTest from tag_summary to change_tag
Bug: T209525
Change-Id: I6ab0b29800d7654164e8d23fb24b81529b0d2c88
2018-11-28 08:04:51 +01:00
Daimona Eaytoy 7427333ed5 Improve code readability
Simplify some logic constructs, reduce the amount of return statements
inside methods, explicitly declare variables before using them, reduce
code duplication, add names to JS anonymous function to produce clearer
stack traces.

Change-Id: Ife4546a91c30d4c519d09a712ba56a2f33abe579
2018-11-19 16:01:37 +01:00
Daimona Eaytoy e055ecc7c6 Reduce code duplication
Change-Id: I03bd56e4bf455865b27338ac39b3dcef20a88447
2018-11-19 15:50:36 +01:00
Daimona Eaytoy 4480c9493a Remove wgParser and wgRequest
As part of the deprecation process of non-config globals.

Change-Id: Ia84ddc20adbfda72347cf256601050b055b87ecf
2018-11-19 13:40:58 +01:00
jenkins-bot 213c2aa011 Merge "Change throttle selector to restore old functionality, overall improvement" 2018-11-15 00:58:11 +00:00
Daimona Eaytoy d3a8491c3f Change throttle selector to restore old functionality, overall improvement
Long (sigh) explanation in T203587#4569698. Also, simplified the way
TagMultiselect are generated, this one and the one for change tags.
This new selector is back-compat both with the old textarea and the OOUI
checkboxMultiselect; actually, this one is //fully// compatible with the
old textarea.
Add validation for throttle parameters and unit tests for validation
(split from I976c95658cddb2585910b6f8a5f047aadc4e4d47).
Added a trim when retrieving throttle identifier to allow syntax like
'ip, user'.
Improved the message shown on history.
Re-added the maintenance script to clean DB.

As I wrote in the task, a review by two other people would be great, at
least for the maintenance script (it could potentially break the DB).

Bug: T203587
Bug: T203336
Bug: T203584
Bug: T203585
Depends-On: I3b2e763bd8835207dc5df1db43d3e1881e6961c3
Change-Id: I7831dbb0bab55807392ac1f7915d6cb0cb713593
2018-11-14 12:51:36 +01:00
Brad Jorsch f6349e7a32 Update tests that fail with comment/actor migration
* AbuseFilterConsequencesTest is somehow leaving blocks behind. Mark
  ipblocks as being used to avoid that.
* AFComputedVariable::getLastPageAuthors() uses indeterminate order for
  multiple revisions with the same timestamp. Fall back to rev_id
  ordering like MySQL accidentally did before.
* AbuseFilterTest tries to create revisions attributed to users that
  don't exist. Switch to interwiki usernames.

Change-Id: I30f7cdcc3875f3f7af116c1e41e88f62ab9e91d0
2018-11-09 17:03:36 -05:00
jenkins-bot 108ec1117f Merge "Reload the test user instance before checking the edit count" 2018-10-23 18:53:33 +00:00
Aaron Schulz 3191c2adc4 Reload the test user instance before checking the edit count
These are updated in deferred updates and should not rely on the same
User instance being used in those updates. This also avoids convoluted
logic in User to set the new edit count for various cases.

Change-Id: I6d239a5ea286afb10d9e317b2ee1436de60f7e4f
2018-10-23 18:06:12 +00:00
jenkins-bot 7e151f5edc Merge "Unbreak short circuit for arrays" 2018-10-18 04:04:31 +00:00
jenkins-bot c2f5540928 Merge "Use fake timestamps for time-related tests" 2018-10-13 22:19:31 +00:00
Daimona Eaytoy cbd57fe7a1 Simplify test parameters
Instead of having lots of huge arrays, use a fixed one and only
overwrite the needed parameters.

Change-Id: I3b2e763bd8835207dc5df1db43d3e1881e6961c3
2018-10-12 16:40:44 +02:00
Daimona Eaytoy 2ad63c95ef Use fake timestamps for time-related tests
With the hope to finally unbreak such tests, making them much more
stable and clean.

Bug: T206501
Change-Id: I275a088b9b21f47892b4e3c4cd11ef8680a9e6d9
2018-10-10 20:26:52 +02:00
Daimona Eaytoy 70b60e5906 Simplify user_age test
This simplifies the test for user_age, although I'm not totally sure it
will be fixed. AFAICS, there's nothing wrong in there, but we'll see on
future phpunit executions.

Bug: T206501
Change-Id: Iee1a2a65d08c2cffc7a0d655be1eadb018d8bf37
2018-10-09 12:46:47 +02:00
Daimona Eaytoy 6d54b83f2c Simplify parser methods
Use a single function to check parameters amount, avoid duplication
between keywordIn and keywordContains, use if...elseif instead of
if-else when statements have a return inside, simplify some other logic,
add typehinting, and change method visibility according to use of such
methods.

Change-Id: I22225a5cbbb93679a0e78bf6e15866829167fbf4
2018-10-03 17:19:40 +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
Daimona Eaytoy d9d5af3890 Unbreak short circuit for arrays
This problem have been making filters potentially fail silently since
2009. Also add tests for arrays to make sure that no problems arise
when short circuit is used.

Bug: T204841
Change-Id: Ie4e2e06498c1202ba73afcc5d164a72427abbca5
2018-10-03 16:44:10 +02:00
jenkins-bot 121df619da Merge "Improve coverage for AbuseFilterTokenizer" 2018-09-09 12:30:49 +00:00
Daimona Eaytoy bffba28713 Add full tests for deprecated variables
This test checks every deprecated variable to be identical to the
newly-named one, and to emit a debug notice. It also changes such debug
to be emitted via logger instead of wfDebug.

Bug: T201193
Bug: T173889
Change-Id: Ie55746bb7731062ae2d46d84857af2a05d78cf4c
2018-08-29 11:00:28 +02:00
Daimona Eaytoy 775c736512 Improve coverage for AbuseFilterTokenizer
This will make tokenizer almost fully covered. The only uncovered parts
are the one with cache and an else condition which I think won't ever be
executed, and thus added a comment for that. Also, remove an obsolete
xxx comment from ComputedVariable (fixed in
I8e420f0259ef6c9e579f7a00beb58f28af9da37d)

Bug: T201193
Change-Id: I6e9a73aa9e437f096f6a1e20d53a7cb50e5ed85d
2018-08-25 10:25:16 +02:00
jenkins-bot 1b5428b9c8 Merge "Improve tests for the AbuseFilter class" 2018-08-23 14:41:00 +00:00
jenkins-bot 826e600731 Merge "Add _age variables to tests" 2018-08-23 14:35:33 +00:00
jenkins-bot 97f98b029d Merge "Improve parser coverage" 2018-08-23 14:33:54 +00:00
Daimona Eaytoy 078e9a3d21 Improve tests for the AbuseFilter class
Add some test cases for conds limit, profiling and other minor things.

Bug: T201193
Change-Id: I9a3035459cafd6537111cf1dea1a2d9a4bd34036
2018-08-23 14:14:57 +00:00
jenkins-bot ad69ea648e Merge "Remove unused function and improve unit test" 2018-08-23 13:46:41 +00:00
Daimona Eaytoy 0e2ae113fb Improve parser coverage
On the way to 100%...

Bug: T201193
Change-Id: I5fd311f861acccb31f346da9acb379b0366488e7
2018-08-23 12:13:47 +02:00
Daimona Eaytoy 03b52c2b37 Remove unused function and improve unit test
AbuseFilterParser::setVars is only used in a parser test. In the past it
was also used in the actual code (see for instance
https://phabricator.wikimedia.org/diffusion/EABF/browse/master/;5cc8dac63ca585c288ca4c8605db810774e39666?grep=setVars), but at the moment it's pretty unuseful.
This patch removes such function and makes the unit test use literals
instead of variables to avoid calling it.

Change-Id: I80cbc4033ff96f2fe8c1da263b1877bfb4c7c0c4
2018-08-23 11:00:16 +02:00
Daimona Eaytoy 90260edad0 Add _age variables to tests
Tests for new variables introduced in
I0993cecc322806382a1b567b60c0a4af69054841.

Change-Id: Iadaa33c20eb26d6e76ac02e3e9c0066b904833bc
2018-08-23 10:50:52 +02:00
Daimona Eaytoy 447d434e2a Improve code coverage
Add some parser tests, improve existing ones, and add missing @covers.

Bug: T201193
Change-Id: I9c0d2d83560baa4a3e1d4465b7919a48c4e26ac1
2018-08-22 19:07:14 +02:00
Daimona Eaytoy d35c42757c Add missing @covers tag
This should help with tracking code coverage and also explains some
coverage discrepancies encountered while writing other tests.

Bug: T201193
Change-Id: I8b20abc46c2d6c6f582953139b9a9f3710b2e4ea
2018-08-22 17:00:38 +02:00
jenkins-bot d94cc34649 Merge "Add deprecated variables to PHPUnit tests" 2018-08-22 12:44:23 +00:00
jenkins-bot a762c82fe7 Merge "Add aliases for "_text" and "article_" variables" 2018-08-22 12:44:20 +00:00
jenkins-bot 777a86314e Merge "Improve code coverage for AbuseFilterParser" 2018-08-22 11:15:00 +00:00
Daimona Eaytoy cd30d5146f Add deprecated variables to PHPUnit tests
Check a bunch of them, they should be computed and be identical to the
ones with new syntax.

Bug: T173889
Depends-On: I5c370b54e6516889624088e27928ad3a1f48a821
Change-Id: I276913a98e06b5f2ff1c5f5f3ba5bcc7b1e8c997
2018-08-22 08:38:31 +00:00
Daimona Eaytoy c962203ad2 Raise tolerance for time-related unit tests to 10 seconds
This helps avoiding failures with tests depending on execution time.

Bug: T202073
Change-Id: I4da859cfb3e49314ca20329e2ad4a3a7c4fae897
2018-08-21 17:18:24 +02: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
Daimona Eaytoy 4f3b020f5d Improve code coverage for AbuseFilterParser
Add some tests and improve others to raise coverage percentage. This
should lead to almost 100% for the AbuseFilterParser class. Aside from
this, a couple of changes:
* Remove an unused function
* Let equals_to_any return a genuine result with empty strings
* Remove an if which will never be true in skipOverBraces, since the
function is called after checking the same conditions.

Bug: T201193
Change-Id: I7020b2ed996236c38c5784d161ad98ec44163406
2018-08-20 14:38:40 +02:00
Umherirrender c954b412c6 Include CheckUser in phan config
Depends-On: I51421184485c3117bbab9ce3dd42f2dbb6c6180c
Change-Id: Ida17580b301ff4a6b0d3d0020c48f65eb1e21026
2018-08-17 17:38:01 +02:00
Daimona Eaytoy bb476e2c45 Fix wrong error message for PHPUnit
We're currently emitting the same error twice, but in one of those cases
it's completely wrong. Damned copy&pasting!

Bug: T202073
Change-Id: I7687826a85f3ef0abaf15d7cd973afc4e55758b2
2018-08-16 17:11:41 +00:00
Daimona Eaytoy 0026a68a8a Add PHPUnit tests for various generic functions
Adding tests for generic functions in AbuseFilter class, ranging from
simple utility function to variable computation.

Bug: T42478
Change-Id: I903fb7ffbc436b27462e3e4611ab65ecb8a543ba
2018-08-09 19:20:46 +00:00
jenkins-bot 4b185b3749 Merge "Add phpunit tests for noparams and notenoughargs exceptions" 2018-08-02 08:41:46 +00:00
jenkins-bot 75729b6195 Merge "Add other phpunit test for AFPUserVisibleException" 2018-08-02 08:07:56 +00:00
Daimona Eaytoy 9440828d13 Add phpunit tests for creating and editing filters
Adding the template for unit tests and some tests. These should cover
all the validation failure cases.

Bug: T42478
Depends-On: Ib7a0335fa7fb3b8a21765438a720205656c1ea09
Change-Id: I3fd0d627295d680ed33b1cbc730435df0446277f
2018-07-18 12:30:55 +02:00
Daimona Eaytoy 5c6007e041 Add tests for filter consequences
The last one of what I think are the must-have tests. This patch
provides the basic tests and the framework, which may be further
expanded later on. Please note that the failures are due to an actual
problem in core, for which there is I7bb0e92b2906a2511fc4290bdc76fc39ec4617fe.

Bug: T42478
Change-Id: I28eb464c63fda7faa3ec7d1f6082f36154d66962
2018-07-15 15:43:18 +00:00
Daimona Eaytoy 4f037c29c2 Add phpunit tests for noparams and notenoughargs exceptions
We're really missing exception tests: in fact, 'noparams' not being
thrown was discovered only a few days ago and worked like that for
years. This patch adds phpunit tests for both noparams and notenoughargs
exception, also checking the returned message.

Depends-On: I484fe2994292970276150d2e417801453339e540
Change-Id: Ia0b9b8fd5c979be06879723b746f9356c628f5cd
2018-07-15 17:35:45 +02:00
Daimona Eaytoy e9921bcda7 Add other phpunit test for AFPUserVisibleException
Follow-up of Iacb8f7a361079e3e117dc6845597c7bd8473e54a for exceptions
thrown outside the parser. With this patch all uses of AFPUserVisibleException
will be covered.

Depends-On: Iacb8f7a361079e3e117dc6845597c7bd8473e54a
Change-Id: Ia7ef6eb832d5725a804a60cb58bc110b06c8abe2
2018-07-01 18:34:01 +02:00
Daimona Eaytoy 7a64280893 Add phpunit tests for all exception thrown in the parser
All uses of "throw" inside AbuseFilterParser are now covered.
Bonus: added a standard suppresswarning when checking regex validity.

Change-Id: Iacb8f7a361079e3e117dc6845597c7bd8473e54a
2018-07-01 18:31:11 +02:00
Daimona Eaytoy c75bc35f7d Rename lists to arrays
Arrays were introduced with the name "lists". While it **may** look
user-friendlier and so on, it actually uses a wrong name: lists are
different from arrays. I ran a grep and I should've replaced
every occurrence, plus everything seems to work, however a double check
wouldn't be bad.

Change-Id: I6a858f02f5dd9250ba7e1abf9c6422fd98758c9e
2018-06-26 14:42:23 +02:00
Huji Lee 2792fce41e Introduce sanitize() function
Normalizes HTML entities into unicode characters

Bug: T169122
Change-Id: Ic916a6f8976e486d62d65156fa2dab56a55cf22a
2018-06-03 16:37:23 -04:00
Daimona Eaytoy caa4b1c763 Add phan configuration
This is taken from I6a57a28f22600aafb2e529587ecce6083e9f7da4 and makes
all the needed changes to make phan pass. Seccheck will instead fail,
but since it's not clear how to fix it (and it is non-voting), for the
moment we may merge this and enable phan on IC.

Bug: T192325
Change-Id: I77648b6f8e146114fd43bb0f4dfccdb36b7ac1ac
2018-04-30 08:32:58 +00:00
Daimona Eaytoy 9eea111d9f Sync parser tests with examples on mediawiki
I added on MW an example of comparison with empty array, which we should
keep inside the dedicated test as well.

Change-Id: Ifa4bca85c8978ef24ed5bb26787730bb4521261f
2018-04-26 18:47:51 +02:00
jenkins-bot 6aa6b8fc13 Merge "Add the remaining equality checks" 2018-04-26 13:25:56 +00:00
Daimona Eaytoy 71f375f19a Add equals_to_any function
Introduce a new function which can be used to group multiple comparisons
in a single condition. In particular, equals_to_any(S, A, B) is the
equivalent of S === A || S === B. This is especially useful in checking
for multiple namespaces, as proposed in the Community health initiative.

Change-Id: I9dcfe303eb5e51e1882fe4a65fa876aa93db7686
2018-04-25 23:12:19 +00:00
Daimona Eaytoy 24c8d7d54e Add the remaining equality checks
I left as ToDo the checks between an array and something else. With this
patch, it'll work like PHP: the result will be true iff the comparison
is loose, the array is empty and the other operand is either false or
null.

Change-Id: Idc5cadb697ed4fc7f4856967274169f77495ed9f
2018-04-25 10:16:50 +02:00
Daimona Eaytoy 3c3a521fec Fix coding conventions exclusion rules
This should fix every error with excluded rules, leaving only the one
for $wgTitle. A double check would be nice in order to avoid regressions
due to stupid mistakes.

Bug: T178007
Change-Id: I22c179f3a01d652640304b59e43fcb5b5a9abac3
2018-04-20 08:40:18 +00:00
Daimona Eaytoy 8cfd527f31 Reinforce parser tests
Some of them are actually too simple, and may be unuseful in tricky
situations. This patch adds a lot of test cases to provide an (almost)
bombproof safety with future patches.

Depends-On: I0bb1ed0109af66997e238b532d342d82d4c4ae19
Change-Id: I274ef306775c36be20acb662353f6537ff3f1a33
2018-04-09 16:25:54 +02:00
Daimona Eaytoy 2dda2e381c Convert division/multiplication/modulo results after calculation
So that type and value will be identical to PHP's ones.

Bug: T191688
Depends-On: I1140900cdda63eed292d9f20aefd721ef9247fcd
Change-Id: I398c9a972b7e9fcb27d055d23939be2b8bb68244
2018-04-09 16:16:04 +02:00
Daimona Eaytoy 284ab234fd Allow comparing two lists
This feature was never implemented. I'm not sure whether we need a way to compare array and other types of variables (left as ToDo), since e.g. in PHP it's always false.

Bug: T179238
Change-Id: I5d2c33fd117e69cbc84c0b04b6cb82edbdcadf16
2018-04-06 11:44:28 +00:00
Max Semenik a5b92a90c0 Fix license header
Change-Id: Ifb6b2d39fab9375e09c22e87ec818d74bd22fb28
2018-04-03 02:16:33 +00:00
Max Semenik 5c89246fce Rename files to match class name
Change-Id: Ia19bfec6c2289912699b6c90261afda311afb56e
2018-04-02 22:08:13 -04:00
libraryupgrader df05002739 build: Updating mediawiki/mediawiki-codesniffer to 17.0.0
The following sniffs are failing and were disabled:
* MediaWiki.Commenting.LicenseComment.InvalidLicenseTag

The following sniffs now pass and were enabled:
* MediaWiki.Commenting.FunctionComment.MissingParamComment

Change-Id: I38c334ea6c6ff07dfcb64d551413a02dc8c5e51e
2018-03-28 23:38:50 +00:00
Umherirrender e01a06df7d Move @group from file comment to class comment
Phpunit is only looking at class comment for annotations

Change-Id: Ic98f5d995051c5fc2a41c3c31b2fdbd39af028b1
2018-03-16 22:00:56 +00:00
Daimona Eaytoy a0de056299 Add contains_all and ccnorm_contains_all functions
Added the contains_all function, with basically the same role as
contains_any but using logic AND instead of OR. Also added
ccnorm_contains_all, that is the same of ccnorm_contains_any but with
AND mode. Finally, fixed three wrong task IDs.
Co-authored with Valerio Bozzolan.

Bug: T21176
Change-Id: Ib0a8b783db6ce0d5db64771c8e0c70f0f8d13d36
2018-02-09 17:33:24 +01:00
Kunal Mehta 5238c8e8b5 Improve @covers tags
Change-Id: I3df3698b5d3f3eae95db8c740c611f365ff9cb31
2018-01-23 14:08:52 -08:00
Daimona Eaytoy 4e20c933f4 Add get_matches function
Added the get_matches function to store a regex match.

Bug: T179957
Change-Id: I19366ebcaa4d0f007dd675a61c91457dde57f604
2017-11-13 17:32:45 +01:00
David Barratt 5335b6c811 Use Equivset library intead of AntiSpoof
Use the new equivset library instead of AntiSpoof.

Bug: T175413
Change-Id: I439387deeba99543e194c210953ac73ff98bc5b7
Depends-On: I977d3498b2084a426e2ab4d85c000d1b9dcfe824
2017-10-21 21:55:18 -07:00
Dayllan Maza 2bc8873c30 Add ccnorm_contains_any function
Normalize and search a string for multiple substrings

Bug: T65242
Change-Id: I4034c0054a6849babbf2d96ea13dc97d3660d5b4
2017-10-06 11:32:45 -04:00
Umherirrender 1a58507870 build: Updating mediawiki/mediawiki-codesniffer to 0.10.0
Change-Id: I5f37c45d748d5f0da21aceaef32cc89367e312ff
2017-07-08 20:49:30 +02:00
Umherirrender a063e33ee8 Use short array syntax
Done by phpcbf over composer fix

Change-Id: I53fd1fc8d056b9b60194d2d630852cfca37aadea
2017-06-15 17:02:57 +02:00
Victor Vasiliev 46faa02c49 Fix the associativity of boolean logic operators
Change-Id: Icaf0fde0d74064532af4b110faef4014f8303f80
2016-11-06 20:30:07 -05:00
Victor Vasiliev aa399da279 Implement a tree-caching abuse filter parser
This filter is fully functional.  The old filter is still enabled by
default for a transitional period in case the new one suddenly has
issues.

Change-Id: I4aea5f00c62420108030e60e79d5bf34e913e95d
2016-09-24 02:53:26 +00:00
Victor Vasiliev 5da98b67bf Add test coverage for more bizzare features of the filter parser
I am pretty sure all of the behavior documented in these tests is a bad
idea.  It is possible that we can fix it since some of those features
are probably unused, but for now those tests will serve as a
documentation of the current behavior.

Change-Id: Ia2a2f57a538d7aef2ac73fb2e47fe82dd5d5e09a
2016-08-21 18:45:22 -04:00
Kaldari acd28cb00f Update tests for AntiSpoof fixes
Bug: T29987
Depends-On: Iccb3e50073bbbc2b979cb62dd0e129afd1c2e55f
Change-Id: I8bef839b9b9ca5fced94ce6428e769133ede868f
2016-08-13 20:37:43 +00:00
Bartosz Dziewoński 5fc30112c7 Optimize 'count()' function
substr_count() is just as fast as looped strpos() when there are no
matches, and gets faster as the number of matches increases.

Note that this introduces a small change in behavior when the needle
is composed of repeated substrings, e.g. 'asdasdasd' or 'aa', and
haystack is such that the needle can be matched in overlapping
positions, e.g. 'asdasdasdasd' or 'aaaaa'. The old implementation
counted overlapping matches, the new one doesn't. I don't think this
behavior was intentional and I don't think this change will cause any
real problems.

Change-Id: Icc905ca34bf08d63e969787a5e3c119d498bf878
2016-04-17 08:32:27 +02:00
Bartosz Dziewoński 7d83540527 Add some tests for behavior of 'count()' function
Change-Id: I29a6c91d0780dc9a1eaee6d29d3b1f9c9c708df7
2016-04-17 08:18:29 +02:00
Bartosz Dziewoński e79b45b71f Improve ignoring short-circuited operations
Previously, 'false & a == b' would actually execute the comparison and
count it against the condition limit, while 'false & (a == b)' wouldn't.
They behave the same now.

mShortCircuit was only checked for the most potentially expensive
operations (computing functions and getting variables), all the other
operations on bogus values generated by this would be executed and the
results ignored later.

This probably doesn't noticeably improve performance, but it corrects
how the condition limit is counted.

Bug: T43693
Change-Id: Id1d5f577b14b6ae6d987ded12689788eb7922474
2016-04-09 16:25:52 +02:00
Bartosz Dziewoński 3b32cf00e9 Improve how the number of conditions is counted
With the new behavior, the number of conditions in incremented when:
* Evaluating a function
* Evaluating a comparison operator (== === != !== < > <= >= =)
* Evaluating a keyword (in like matches contains rlike irlike regex)

Previously, the number of conditions was incremented when:
* Evaluating a function
* Entering the comparison operator evaluation mode

This resulted in a number of surprising behaviors. In particular:
* '(((a == b)))' counted as 4 conditions, not 1
* 'contains_any(a, b, c)' counted as 5 conditions, not 1
* 'a == b == c' counted as 1 condition, not 2
* 'a in b + c in d + e in f' counted as 1 condition, not 3
* 'true' counted as 1 condition, not 0

It is still possible to easily cheat the count by rewriting comparisons
as arithmetic operations. I believe this is meant to advise users of
the complexity of their rules and not really enforce strict limits.

Bug: T132190
Change-Id: I897769db4c2ceac802e3ae5d6fa8e9c9926ef246
2016-04-09 16:16:27 +02:00
Ori Livneh bab9832415 Move rule tokenization to new AbuseFilterTokenizer class
* Move AbuseFilterParser::nextToken() and the various AbuseFilterParser
  properties that accompanied it to a new class, AbuseFilterTokenizer.
* Tokenize rules eagerly and cache the result in APC.

Change-Id: I15f5b5b65e8c4ec4fba3000d7c9fd78b98967d1d
2015-08-25 14:00:10 -07:00
Ori Livneh b388dfab1b Clean-up of AbuseFilterParser::nextToken()
No functional changes.

* Don't include $code as part of the return value; it is ignored anyway.
* Removed AbuseFilterParser::lastHandledToken and AFPParserState::lastInput,
  because AbuseFilterParser::nextToken() no longer calls itself recursively.
* The regular expression that matches operators is no longer constructed
  dynamically, but hard-coded into the class. To make sure it does not drift
  apart from the more legible AbuseFilterParser::$mOps, add a unit test that
  constructs the regex dynamically as before and compares it to
  AbuseFilterParser::OPERATOR_RE.
* AbuseFilterParser::RADIX_RE ditto.

Change-Id: I9c23b60759ed2f4c73a9b480243b16bbce5a208f
2015-08-25 10:50:31 -07:00
Ori Livneh 0e36b728e3 Fix double escaping in AFPData::keywordLike()
If we don't map '\-' and '\+' to themselves, the leading slash gets escaped,
and the resultant pattern only matches a literal slash.

Bug: 67670
Change-Id: Ifa1e3edd6f41985a3bb97bfb1497985f8fa64af5
2014-07-11 14:56:42 -07:00
Marius Hoch 35747761fb Allow running the AbuseFilter parser tests via phpunit
I've also added myself to the credits file as I'm the only
maintainer of this extension for a while now.

Change-Id: Id998172ea2abd70b8243de9db1a96cc2cfa47a64
2013-07-08 19:22:43 +02:00
jenkins-bot 3c83358506 Merge "Add parser tests for bug 25373" 2013-05-01 21:25:11 +00:00
Kunal Mehta 4bec58cd54 Add a "ucase" function to convert the provided string to uppercase.
I basically took the lcase code and tweaked it to work for uppercase.

Bug: 47321
Change-Id: I230dbd99c27bf3a4a042befd6d334b4c0439bde0
2013-04-17 11:48:15 -05:00
Marius Hoch 3010d78950 Add parser tests for bug 25373
Change-Id: I2f2524731098f323e61bbc0442e7b56b11cdea37
2013-03-23 21:49:57 +01:00
Marius Hoch 03da29b9da Fix the abusefilter array parser test
The abusefilter array test failed because length( ['a', 'b', 'c'] )
returned 12 instead of 6. That was du to it converted the array
to a string with new line seperated values first before measuring
the string length. Changed that behaviour to act like the php count()
function or the python len() function which seems far more useful to me.
The old behaviour can be established using length( string( array ) ).

Change-Id: I16646891837c9743ca5af2dd328077a7225bb5f1
2012-12-20 02:19:55 +01:00
Alexandre Emsenhuber 56e6f0a262 svn:eol-style native 2009-04-09 20:45:31 +00:00
Victor Vasiliev 27fb1303a8 * Use lists instead of implode()d strings in built-in variables wherever it's possible
ATTENTION! This may break filters that rely on "added_lines contains 'bla-bla'" syntax. They'll need to be replaced with "string(added_lines) contains 'bla-bla'"
2009-04-05 19:07:47 +00:00
Victor Vasiliev 128ae5983b Introduce list (non-associated array) support into abuse filter parser. 2009-04-05 17:11:17 +00:00
Victor Vasiliev 258d340fb5 Abuse filter:
* Introduce := operator for setting variables
* Throw an exception when user tries to override built-in variable
* Fix UTF-8 handling in fnmatch() fallback
* Copy three main abuse filters from enwiki to test suite
* Fix update.php integration
2009-04-05 11:47:42 +00:00
Andrew Garrett 86e4081206 Abuse Filter Parser:
* Efficiency -- use /A instead of PREG_OFFSET_CAPTURE and comparing offsets.
* Expand error messages to enhance debugging.
* General code quality
2009-03-25 11:36:38 +00:00
Andrew Garrett 0880f444b1 Abuse Filter Parser updates:
* Use strcspn to scan ahead for long regions of uninteresting text in string handling (performance).
* Remove cruft specific to my system in phpTest.php.
* Remove a test that was in incorrect syntax, and useless without adding variable support.
2009-02-11 18:23:21 +00:00
Andrew Garrett bfe57be65d Rewrite of Abuse Filter parser tokeniser.
I've made it more performant and fixed a few bugs by using regexes
instead of PHP loops, where possible, under the assumption that the
PCRE parser is more efficient than the same thing implemented in pure PHP.
Also, I'm now passing the same string around and calculating offsets, which
Tim tells me is far more performant than continually truncating the same string.

All tests still pass, with the exception of string.t, which I've modified
to remove the offending code, which never worked.
2009-02-11 01:41:51 +00:00
Andrew Garrett 53179c675f Apply changes from change-tagging branch. I will remove all of the stuff actually related to change tagging in a moment, to avoid trunk changes on Wikimedia sites. 2009-01-23 19:23:19 +00:00