This was a means to bypass the limitation to filter by
triggered filter (for example, when a group contains
a single filter).
Change-Id: Icd7b0b64ff16b4ce26f4d52ad9d9abce62972e60
The array_filter is likely meant to empty the array if the empty string
was exploded ( `explode( "\n", '' ) === [ '' ]` ). However, it can also
remove other stuff, e.g. the string '0'. An explicit comparison is
easier to read & interpret, marginally faster, and avoids rare but not
impossible edge cases.
Change-Id: Ie77d65b56319664a2ac370f32341dc72b619a635
Previously, the cached value would depend on the tags
parameter to be updated. The provided value may be
different for each call, so callers may receive
unexpected values.
For example, while core usually calls this with core-defined
hooks, our method AbuseFilter::isAllowedTag calls this
providing an empty array. If core's call happened shortly
after ours and hit cache, its array would be overwritten
with only AbuseFilter's tags, the rest would be lost.
Also do some clean up:
- only call array_filter on explode'd array
- call array_unique on the value, since it's usual that
multiple filters share the same tag
Noticed when thinking about moving this to a service.
Change-Id: I4f4322e80ec89e48458a3bf46a1146863bec8237
Copying my investigation from I8c93e2ae7e7bd4fc561c5e8490ed2feb1ef0edc2:
This code was introduced in 2009, see rEABF0f1eb8db78bfa83ddb93427f39aad619523d8f25:
$display = wfMsg( "abusefilter-action-$action" );
$display = wfEmptyMsg( "abusefilter-action-$action", $display ) ? $action : $display;
And wfEmptyMsg looked like this:
function wfEmptyMsg( $msg, $wfMsgOut ) {
return $wfMsgOut === htmlspecialchars( "<$msg>" );
}
so this made sense. But then, in 2010 (rMWae3ced88e535c7fd046f0ad6f0710cc87f0004ea) the function was changed:
function wfEmptyMsg( $key ) {
global $wgMessageCache;
return $wgMessageCache->get( $key ) === false;
}
without anyone removing the parameter from AbuseFilter.
Finally, in 2012 (rEABF176227e721c9475de2c2163d3b6e20ca4769c406) the usage of wfEmptyMsg was removed, and $display became a parameter to wfMessage().
Long story short, no need to pass that parameter.
Change-Id: Iad875f0c0ab5aaa06c795232638f52e9ca62786e
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
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
- 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
This is a simple change but with tons of benefits:
- Easier to track usages for IDEs
- Easier to understand in static analysis (phan)
- Can be analyzed by phpda
- Ensures no typos
- These classes can be namespaced without affecting readability here
Change-Id: Ic04d19dfbe9184baf2ef4bac53011521e2e44953
- Use null instead of empty strings
- Check the mode, and not the pattern, to decide whether the user
searched for something
- The call to parent::__construct can now be moved up
- Note in a comment how this code is problematic due to "smart"
constructors
- Avoid caching the headers, as that's not going to work anymore.
Change-Id: I420ab0215d53354a67d9d130ebd8d85dfbd2778b
These have been saved in the parent class for quite some time.
Refactor accessors in method overrides.
Change-Id: I9819caa5ab87ac3a8e47efb32b00d89c3e2a61af
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
When the user selects to see global rules and it's a remote wiki, hide the rule search field. (Note that the list of search modes needs to listen to this setting as well.)
This was discussed during reviewing I0771fa048.
Also move local/global filters setting to the top as it's more important than that for disabled and deleted filters (which will both stay together).
Change-Id: I0912aa1f5d7a5d75e6ae5a2a3362b8d38260c611
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
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
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
This reverts commit 6a268e7339.
Reason for revert: Ic1252efe9f96743d9402fa31a7b2dca1f57ff6ae ended up not renaming the index, so this patch removed an index that was still in use.
Change-Id: Ide4a600a57bcfa4da0c7354b972cc89709ccd660
This fixes the abuse_filter_log patch-afl_change_deleted_patrolled
not being applied. The patch is provided for (and should work with) all
the supported DBMS.
Additionally, fix the base table files, which would report
afl_patrolled_by as 'NULL', whereas on the WMF cluster it's 'NOT NULL
DEFAULT 0'. The schema patch takes care of converting that column as
well.
Note that this schema change needs not be applied on the WMF cluster, as
that's already up-to-date.
Finally, note that this patch must be backported to 1.33 and 1.34 (and
it might be fairly hard due to the recent schema changes on the
abuse_filter_log table).
Bug: T240895
Change-Id: Ibdbc9b50c25b9e871ebdeae93a54d10877b585f8
The <pre> element is now hidden with CSS, and is only shown after the
user clicks the "Eval" button.
Moreover, make the button primary and progressive, as to indicate that
it activates the primary function of that page.
Bug: T253492
Change-Id: I300ce6ec0a84ea73025a5af9173024df7c291e03
We have many topnav links, and future patches may add others (e.g.
Ia5fd4f0b35fcabf045a7b49fa40fa85b72c92544). The "import" feature is
probably the less used, and is also pretty similar to creating a new
filter.
Thus, remove its link from the topbar and move it to a button next to
the "Create a new filter" button.
Note that the old message is reusable, and thus it should be moved on
translatewiki after merge.
Change-Id: I52042d62b2bab7e4a1e9bbc027e7de5addec8157
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
