Commit graph

900 commits

Author SHA1 Message Date
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
Ammar Abdulhamid 323fe4666c Draw suppression reason from Revdelete-reason-dropdown-suppress
Bug: T245990
Change-Id: Ic5adcf4e6693cb5b4c849156e54d97cf35b70dee
2020-02-24 14:06:43 +01:00
jenkins-bot 76a1be97a4 Merge "Add site name and language variables" 2020-02-10 19:06:01 +00:00
jenkins-bot b6ca1402d0 Merge "Rename addStaticVars and related hook" 2020-02-10 19:03:34 +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 57415d8a31 Fix PHPNotice caused by missing row fields
Follow-up Ie9aae938cca06e38a7a834a3f74f3e8735ab01ee.

Some fields are actually necessary when the filter isn't saved. This
would cause PHP notices when showing the editor again.

Change-Id: I2b9e0f04b3e8ad4eea8e334e16ee422bb40f0eb5
2020-02-09 13:36: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
Daimona Eaytoy 661a77f0eb Rename addStaticVars and related hook
This code was introduced with Iba59fe8d190dd338ecc8cfd682205bce33c9738b
and is unused since then. The name should highlight that those variables are not
supposed to be "static", i.e. immutable. Examples are: timestamp, spam
blacklist, site name, site language. These are not immutable, but rather
"generic", and they're known even without an ongoing action.

Also add an RC row param and update docs.

Change-Id: I402f04585e9154059fc413e527e39dcb8e6b3d7c
2020-02-09 14:29:08 +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 6b7be78534 Use RCDatabaseLogEntry as wrapper in get*VarsFromRCRow
This provides various shortcuts for user, target, comment, etc.,
avoiding direct access to the row, and thus a dependency on the
schema.

Change-Id: I250f94e0ac6cade33441a31ae8a27093a4d937a0
2020-02-07 19:19:10 +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
libraryupgrader a14ec744f7 build: Updating composer dependencies
* mediawiki/minus-x: 0.3.2 → 1.0.0
* mediawiki/mediawiki-phan-config: 0.9.0 → 0.9.1

Change-Id: I119f4d56cce674302f34e938e598e6cc6bf28dc0
2020-01-28 17:51:38 +00:00
Ammar Abdulhamid 641aeebbcf Replace deprecated IP class with IPUtils
Bug: T242556
Change-Id: If8e9034885726b673d1500fa8b538b5302e66165
2020-01-24 18:27:26 +01: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
James D. Forrester bdef1200f8 Follow-up 87459ec: When no registration date is recorded, use 2008-01-15
Before the phan upgrade, this was silently choking on null as so falling
back to age since 1970-01-01 (~50 years); since the upgrade, the code is
breaking filters by responding with 0. The approximation of using 2008's
Wikipedia Day is less wrong and more fun (credit to Roan for making this
suggestion).

Bug: T243469
Change-Id: Ibc25ab09ecd0bf0b2292425c2768b1dc911b9974
2020-01-22 15:38:09 -08:00
jenkins-bot 5db1032618 Merge "Simplify throttling code" 2020-01-22 17:19:52 +00:00
jenkins-bot 81fd6af030 Merge "Actually record all filters in total_filters" 2020-01-22 17:19:50 +00:00
jenkins-bot 70d31da673 Merge "Stop using deprecated stuff with easy replacements" 2020-01-22 16:44:57 +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
libraryupgrader 1d911b8187 build: Updating dependencies
composer:
* mediawiki/mediawiki-codesniffer: 28.0.0 → 29.0.0
  The following sniffs are failing and were disabled:
  * MediaWiki.Commenting.FunctionComment.MissingParamTag
  * MediaWiki.Commenting.FunctionComment.ParamNameNoMatch

npm:
* eslint-config-wikimedia: 0.13.1 → 0.15.0
* grunt-stylelint: 0.11.1 → 0.13.0
* stylelint-config-wikimedia: 0.6.0 → 0.8.0

Additional changes:
* Remove direct "stylelint" dependency in favor of "grunt-stylelint".
* Also sorted "composer fix" command to run phpcbf last.
* Removing manual reportUnusedDisableDirectives for eslint.

Change-Id: I8f73202db1333fbc36ccf556b3bb05b1e8c279cb
2020-01-21 07:38:54 +00:00
Daimona Eaytoy 87459ec679 build: Upgrade phan
Depends-On: I6d538ce3ca7fd2d495c2bafbab7cc279da69db1c
Change-Id: Ic8c3a01a5c37fdf461f4fd5598e597eb9c9073d3
2020-01-19 18:48:51 +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
Daimona Eaytoy 10c2fe7151 Stop using deprecated stuff with easy replacements
This patch is mostly replacing Revision::* constants,
Wikimedia\(restore|suppress)Warnings, and wfWikiId.

Change-Id: I13544cc3e12955a9376ccce3c120e2cee1f2ee2e
2020-01-08 14:59:30 +01:00
jenkins-bot f0e4c22b53 Merge "Simplify a query in AFComputedVariable" 2020-01-07 19:30:20 +00:00
Daimona Eaytoy c54e2fc180 Simplify throttling code
Change-Id: I54ebdf0bc61d5628d1755b75232a934358b112ff
2020-01-07 17:52:16 +01:00
jenkins-bot eef2760d7b Merge "Use explicit variarg for VariableHolder functions" 2019-12-27 10:24:31 +00:00
jenkins-bot 8fea62529b Merge "Fix AbuseFilterCachingParser violating return type constraint" 2019-12-27 10:04:57 +00:00
jenkins-bot a46c0e7359 Merge "Restore the ability to filter content model changes" 2019-12-27 10:02:50 +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 d43756a7f4 Merge "i18n: Rename msg key for abusefilter-view-oldwarning" 2019-12-23 12:16:57 +00:00
jenkins-bot db3b4703c5 Merge "Don't use mFilter in ViewTestBatch" 2019-12-23 12:11:52 +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 a7dd20b040 Don't use mFilter in ViewTestBatch
In other View* classes, AbuseFilterView::mFilter contains the ID of a
filter, e.g. the filter being edited in ViewEdit. In ViewTestBatch,
however, it is a string containing some filter text. Hence, use a new
private property instead (without the legacy "m" prefix).

Change-Id: Ib22ce238aff4ca5ed57ba725ee9bff7f8c3d153b
2019-12-16 12:17:49 +01:00
Daimona Eaytoy b814c0827a i18n: Rename msg key for abusefilter-view-oldwarning
Thinking about it again, all messages on ViewEdit start with
abusefilter-edit. Also add a reference to the other message to
facilitate translations.

Follow-up: I3717d06d4a757684fe6622961391ae06b5bd3c38
Bug: T235590
Change-Id: I4cbaa2e92d22296f55a4b5ef0c633fe959fe9ea3
2019-12-16 10:56:30 +00:00
DannyS712 12efe4a0ad ApiAbuseLogPrivateDetails: private-details should be privatedetails
Bug: T240812
Change-Id: I263e3a57a48ab6a58e4c7f2181a914d9800a2fc5
2019-12-16 03:25:15 +00:00
Daimona Eaytoy f382304aae Add a base class for parser transition
Change-Id: I31282b8632c332b6d46a6bb4a42f57ac0d005b5f
2019-12-15 13:29:56 +00:00
Daimona Eaytoy c432f058fd Restore the ability to filter content model changes
Follows-up I3fb7b36ab38ca1544889a4c233b8ffdfc6c80936

Bug: T240485
Change-Id: I2e8337e6f505932a18a5bb5a0d97b9d6bc3f42c8
2019-12-11 19:42:45 +01:00
Daimona Eaytoy 20c6810039 Use explicit variarg for VariableHolder functions
This is easier to read and to document, and it also allows typehints.

Change-Id: Ibd6642aa26e25a785faadf5139e64ea884ff4de2
2019-12-10 19:28:07 +00:00
Daimona Eaytoy 6be070e5a2 Strengthen the check for null edits
Even if the Content objects are different, the normalized text contents
may be identical.
Also, stop misattributing null edits by adding the last revision of the
page as afl_rev_id.

Bug: T240115
Change-Id: I3fb7b36ab38ca1544889a4c233b8ffdfc6c80936
2019-12-10 17:03:49 +01: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
jenkins-bot 357dcdce5c Merge "SECURITY: Unbreak blocks shorter than one hour" 2019-12-04 18:17:58 +00:00
Daimona Eaytoy 759cb38bf5 SECURITY: Unbreak blocks shorter than one hour
Bug: T238768
Change-Id: I8820a6e2953255f409ff8ddc2b41dcef2f338e37
2019-12-04 18:46:40 +01:00