Commit graph

156 commits

Author SHA1 Message Date
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
jenkins-bot 903f3db8fe Merge "Beautify old, broken abuse_filter_history rows" 2019-04-10 05:11:38 +00:00
Daimona Eaytoy 25ed009518 Beautify old, broken abuse_filter_history rows
And right when the throttle script seemed complete... Here is another
function! So, this change splits the logic in new functions called
sequentially, and the only actual change is adding the beautifyHistory
function. Its purpose is to search ANY row in abuse_filter_history with
empty/missing parameters and normalize it. More specifically, missing
period and count are inserted as 0, and for missing groups we add
"none", used by a newly introduced message. This way, messages shown on
Special:AbuseFilter/history will be clearer and won't have gaps.

Bug:T209565
Bug:T215787
Change-Id: I38395f4df9d83badfd26cdf584ffba743b6417a9
2019-04-10 04:51:58 +00:00
jenkins-bot efe32b7c93 Merge "Add doc for every class member" 2019-04-06 14:37:19 +00:00
jenkins-bot e03488b66a Merge "Overhaul tag selector" 2019-04-06 12:35:20 +00:00
Daimona Eaytoy a777b681e2 Don't publish LogEntries without ID
Mimic what publish() does, for the part that we need.

Bug: T219951
Bug: T218940
Change-Id: I16dd7071837a6965934d08b770f455f45cd02a6b
2019-04-06 09:46:09 +02: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 c095069918 Log throttled actions
So that we have some numbers and debugging info.

Change-Id: I490504bfac09d537be5bca36ef52992b6aa4f0cb
2019-03-23 11:52:48 +01:00
jenkins-bot 77a0fd6c0d Merge "Replace double-equals with triple-equals" 2019-03-23 07:56:37 +00:00
Holger Knust 324d0e6aa3 AbuseFilter: Swapped getNative for getText
Use getText method in AbuseFilter.php instead of deprecated getNativeData method.

Bug:  T155582
Change-Id: Ia1af3abea8331e2292d608215946834b282ed753
2019-03-22 17:48:19 -04: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
jenkins-bot 44d602b9a9 Merge "Use lowercase for built-in variables" 2019-03-22 10:44:24 +00:00
jenkins-bot 85b16f1ecd Merge "Change method used for file_bits_per_channel in /test" 2019-03-22 10:34:11 +00:00
Daimona Eaytoy 45e84bad0e Change method used for file_bits_per_channel in /test
Using MWProps will not raise an 'undefined index' notice and, if the
bitsdepth cannot be established, will return 0. This is way better than
returning the empty string or null, which cannot be treated as integers.

Bug: T218874
Change-Id: I2b585f3a2f257783c15f0fcdd2851f1e1ce4256b
2019-03-21 14:05:54 +01:00
Daimona Eaytoy 4103ff56af Use 'page' prefix for upload vars
Rationale on phabricator.

Bug: T218873
Change-Id: I33128098c186ab716951c72e0d917663721b4740
2019-03-21 13:29:43 +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 bae9c5bb8f Use lowercase for built-in variables
The uppercase is just a leftover from a long time ago. Currently,
variables are case-insensitive, and we already perform a strtolower when
saving them. Since most parts of the code already use lowercase, the
uppercase leftovers only make it harder to grep the code to find
variables. As a bonus, make Ace recognize variables in a
case-insensitive fashion.

Change-Id: I72933fcc9952fc1aabf6464b2fc0b04ec39c024b
2019-03-17 14:23:11 +01:00
jenkins-bot ef8c8e6006 Merge "Make the filter field on ViewHistory of "int" type" 2019-03-17 12:00:47 +00:00
jenkins-bot dc0c5cc5ac Merge "Make uploads testable" 2019-03-17 11:06:43 +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 f63b7e8039 Merge "Remove the hacky 'context' variable" 2019-03-17 10:35:31 +00:00
Daimona Eaytoy 3211c71739 Make uploads testable
This patch adds missing methods for testing a filter against uploads in
RC. Please note that (as discussed below) using wfFindFile could be
relatively expensive (as it will be executed for 0-100 RC rows).
If this is true, then we should either use another method (but I
couldn't find a suitable one), or simply reduce the amount of testable
uploads to a reasonable limit.

Bug: T170249
Change-Id: Id406d4e1571873f49bb11e69029311b24ececf49
2019-03-17 11:32:56 +01:00
jenkins-bot de4e971c72 Merge "Remove usage of MakeGlobalVariablesScript hook" 2019-03-17 10:32:30 +00:00
jenkins-bot 4c63c9190c Merge "Remove deprecated DB method" 2019-03-17 10:08:50 +00:00
jenkins-bot e2f1880922 Merge "Don't use wgLang and wgContLang" 2019-03-17 09:53:16 +00:00
Daimona Eaytoy 7449b43c2f Remove deprecated DB method
Database::nextSequenceValue is deprecated and just returns null.

Change-Id: I8109e90ab836b5915873da26da87ea9225e2ff03
2019-03-17 09:38:46 +00:00
Kunal Mehta cb1458f91e Fix parameter order in doc comment
Spotted by phan 1.2.4.

Change-Id: I63aff03d48f1749e03d3398016ead01bc37fe73d
2019-02-23 21:29:39 -08: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
Daimona Eaytoy 27f8b9ab34 Make the filter field on ViewHistory of "int" type
Following up I636b4e56f39282593c737ace1d6ff2d90900d997, enforce a basic
clientside validation and don't fill the field with the URL parameter if
it's not valid.

Change-Id: If4fd015dff64237375a0c4d3b9fbcefbd54dba3e
2019-02-10 12:11:52 +01:00
Daimona Eaytoy 85ba973747 Move the "global-" prefix to a const
And add an utility function to use it to build full names.

Change-Id: Ib5fdeb75c1324f672b4ded39681f006fde34b4d1
2019-02-06 14:42:05 +01: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 ba1b27d7f6 Optionally pass the filter ID to checkConditions for error reporting
Now that Parser errors are on logstash, I noticed a huge spike of errors
on Wikimedia Commons, about 35000 per hour. They seem to be due to 2
broken filters, but id doesn't say which ones.

Change-Id: I8510319c075520f9a893cd7d56f2e30679e249ba
2019-01-24 10:03:52 +01:00
Daimona Eaytoy a207cf22f7 Unbreak tagging for createaccount actions
Tagging doesn't work for account creations, and probably never did. This
is because we used a wrong identifier for such actions. This patch fixes
the problem, although in the long term we should find a smarter way to
apply tags.
Also, clean AbuseFilter::$tagsToSet if the action will be prevented.

Depends-On: Ia8e38ba25d1989fe71714d2b76891c4587921466
Change-Id: I8edcca17ecdcf71397cc9b0d101e8b13ac112047
2019-01-23 21:25:47 +00:00
Daimona Eaytoy f3f8bd11b9 Re-execute checkAllFilters if the edit was stashed
This may solve several issues, see T176291#4105438 for further details.

Bug: T191430
Bug: T176291
Depends-On: Iebbdeac7898b35beea79aa3d0cdf9d0fb265d726
Change-Id: Ia8e38ba25d1989fe71714d2b76891c4587921466
2019-01-23 18:16:01 +00:00
Daimona Eaytoy bc875d8002 Fix SQL key
When updating the abuse_filter_history table, the sequence to use is the
one on afh_id... And we were using the af_id one since 2009.

Change-Id: I3e291c780119d74be5f47e745a8de13bda85486b
2019-01-23 16:24:02 +01:00
Daimona Eaytoy fca80fa976 Remove usage of MakeGlobalVariablesScript hook
This is an old leftover, used to add global JS variables in a convoluted
way: using a hook and a total of 3 static properties. We can safely
remove all of this and just call OutputPage::addJsConfigVars, which BTW
is already called where we need it.

Change-Id: Ifad0618fa93b0c7a7e8b23f596234e622aa8846a
2019-01-21 14:27:57 +01: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 41f6a85a42 Merge "Rewrite the method for getting a global emergency value" 2019-01-19 13:25:41 +00:00
jenkins-bot f8b5965ff9 Merge "Expand AbuseFilter::getFilter to select all fields and fix caching" 2019-01-19 13:17:16 +00:00
jenkins-bot a7955a5142 Merge "Move a method out of AbuseFilter.php" 2019-01-19 12:22:39 +00:00
jenkins-bot b44984c50a Merge "Remove unused stuff" 2019-01-19 12:18:22 +00:00
jenkins-bot 91e1833bc0 Merge "Fix topnav links" 2019-01-19 12:11:07 +00:00
jenkins-bot 575646393b Merge "Improve code readability" 2019-01-19 12:11:06 +00:00
jenkins-bot 7f62874658 Merge "Change method visibility for AbuseFilter class" 2019-01-19 12:02:51 +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 34d3f9acb2 Fix topnav links
*Don't reuse a message (which is bad), instead add a note for
translators. We can also move it on translatewiki.
*Don't show the AbuseLog link if the user cannot see the AbuseLog.

Change-Id: I4ce73b2160275fdc4b0b7bec722471696d8c6a4d
2019-01-17 15:09:29 +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 f700139215 Remove the hacky 'context' variable
First step for removing meta-variables, the second one being removing
global_log_ids and local_log_ids.

Change-Id: I01cd79771c0ee0865abaef6757a930aacd8138d2
2019-01-05 18:30:37 +01:00