Commit graph

533 commits

Author SHA1 Message Date
Daimona Eaytoy 6684ea6450 Remove AFPTransitionBase
Also cleanup the mPos hack in the CachingParser.

Change-Id: Ib5693802a3ceb80cb736880ed65e27340abef689
2021-09-06 19:33:48 +00:00
jenkins-bot 199cf1edf8 Merge "Add a static analyzer for the filter language" 2021-09-03 19:51:58 +00:00
Matěj Suchánek 0af21948fc Replace WikiPage::factory in non-test code
Change-Id: I1442ca6603ce5151b98fc88cd84c25af0f34e4f6
2021-09-01 04:55:25 +00:00
Daimona Eaytoy 86257d825c tests: Use DBConnRef, not IDatabase, as retval of getConnectionRef
So that the method can be typehinted in core.

Also add phan-var to fix broken master build due to typehint additions
in core.

Change-Id: I4a072e00ffeeb437753fc3d3c1f15de9929df510
2021-08-31 21:45:10 +02:00
Sorawee Porncharoenwase 320e3d696f Add a static analyzer for the filter language
This commit adds a class AFPSyntaxChecker which can statically analyze
a filter code to detect the following errors:

- unbound variables (which comes in two modes: conservative and liberal,
  default to conservative)
- unused variables (disabled by default for compatibilty)
- assignment on built-in identifiers
- function application's arity mismatch
- function application's invalid function name
- non-string literal in the first argument of set / set_var

The existing parser and evaluator are modified as follows:

- The new (caching) evaluator no longer needs to perform variable
  hoisting at runtime.
  - Note that for array assignment, this changes the semantics.
- The new parser is more lenient, reducing parsing errors.
  The static analyzer will catch these errors instead, allowing us
  to give a much better error message and reduces the complexity of
  the parser.
  * The parser now allows function name to be any identifier.
  * The parser now allows arity mismatch to occur.
  * The parser now allows the first argument of set to be any expression.

Concretely, obvious changes that users will see are:

1. a := [1]; false & (a[] := 2); a[0] === 1

   would evaluate to true, while it used to evaluate to the undefined value
   due to hoisting

2. f(1)

   will now error with 'f is not a valid function' as opposed to
   'Unexpected "T_BRACE"'

3. length

   will now error with 'Illegal use of built-in identifier "length"'
   as opposed to 'Expected a ('

Appendix: conservative and liberal mode

The conservative mode is completely compatible with the current evaluator.
That is,

false & (a := 1); a

will not deem `a` as unbound, though this is actually undesirable because
`a` would then be bound to the troublesome undefined value.

The liberal mode rejects the above pattern by deeming `a` as unbound.
However, it also rejects

true & (a := 1); a

even though (a := 1) is always executed. Since there are several filters
in Wikimedia projects that rely on this behavior, we default the mode
to conservative for now.

Note that even the liberal mode doesn't really respect lexical scope
appeared in some other programming languages (see also T234690).
For instance:

(if true then (a := 1) else (a := 2) end); a

would be accepted by the liberal checker, even though under lexical scope,
`a` would be unbound. However, it is unlikely that lexical scope
will be suitable for the filter language, as most filters in
Wikimedia projects that have user-defined variable do violate lexical scope.

Bug: T260903
Bug: T238709
Bug: T237610
Bug: T234690
Bug: T231536
Change-Id: Ic6d030503e554933f8d220c6f87b680505918ae2
2021-08-31 03:28:24 +02:00
Daimona Eaytoy 704364a5e7 Move parser exceptions to specific namespace and rename them
Create a dedicated "Exception" sub-namespace and remove the "AFP"
prefix, a leftover from the pre-namespace era.

Change-Id: I7e5fded9316d8b7d1628bc1a6ba8b1879ac901e1
2021-08-29 23:38:31 +00:00
jenkins-bot 9b93b0256a Merge "Avoid passing invalid offset to mb_strpos" 2021-08-18 18:45:12 +00:00
Daimona Eaytoy e9795468c4 Switch filterable actions hooks to the new system
Bug: T261067
Bug: T211680
Change-Id: I0e7e4a48b56c3e5fde56f50693fd0cdc19c30dd0
2021-08-16 14:18:56 +00:00
TChin bfa72b9caf Use MovePageFactory
Bug: T252934
Change-Id: I39440ef05d9318f9ab4abd34990887971197a045
2021-08-10 16:31:05 -04:00
Matěj Suchánek ace6f652af AbuseFilterConsequencesTest: Don't call non-static method statically
Change-Id: I0b4ed2f456bf4a52756eb0b98a29994a4a53812c
2021-07-30 01:24:15 +00:00
libraryupgrader 5377ebe819 build: Updating dependencies
composer:
* mediawiki/mediawiki-codesniffer: 36.0.0 → 37.0.0

npm:
* postcss: 7.0.35 → 7.0.36
  * https://npmjs.com/advisories/1693 (CVE-2021-23368)

Change-Id: I2b382f3bb236fb44eb24c6a257b13b8fd886541c
2021-07-21 18:51:18 +00:00
DannyS712 745d911d68 Add tests for afl_rev_id being set
Regression tests to make sure T286140 does not
happen again.

In the process, discovered what caused that bug
with afl_rev_id not being set: EditRevUpdater::updateRev()
compares the WikiPage given in the PageSaveComplete hook
to the one given to it by AbuseFilterHooks from
onEditFilterMergedContent, and compares the two using
`===`, meaning that they must refer to the same underlying
object. That bug was caused because AbuseFilterHooks
changed to providing a different object, despite still
referring to the same underlying page.

We should probably change that behavior in EditRevUpdater,
but for now updated AbuseFilterConsequencesTest to pass
the same object around by using RequestContext::setWikiPage()
and providing the WikiPage object to
MediaWikiIntegrationTestCase::editPage().

Bug: T286140
Change-Id: I6562f513c463538af6b59b12a64564b254024613
2021-07-04 08:04:06 +00:00
Daimona Eaytoy 069fa064f5 Avoid passing invalid offset to mb_strpos
Bug: T285978
Change-Id: I3d100fd05f34fe3b01ecbbce5361badc613f9406
2021-07-02 14:07:46 +00:00
DannyS712 47f861b6f6 Pass a user to WikiPage::prepareContentForEdit()
Bug: T285447
Change-Id: I4d277419106c3af5222377a863c80dd866ba188b
2021-06-24 04:01:33 +00:00
jenkins-bot 805ea5b4ff Merge "User mock must return Block instance from getBlock." 2021-06-08 19:14:52 +00:00
jenkins-bot 405e6a7771 Merge "selenium: Update wdio-mediawiki" 2021-06-08 17:35:08 +00:00
daniel 54285fe984 User mock must return Block instance from getBlock.
Change-Id: I569e91dd07b8f89af42344b6d6df87560dcb6bbe
Needed-By: T271494
2021-06-08 17:12:48 +02:00
jenkins-bot 997e665530 Merge "Don't use p class="success" for success messages" 2021-06-04 08:59:58 +00:00
Roman Stolar c347a0d33e Update DatabaseBlock construct option 'by' to use User Identity only
Bug: T283641
Change-Id: I16b07e18441143a5d5d470eef4c28037a150b605
2021-05-31 17:36:55 +03:00
sahil 5fca7c2f51 selenium: Update wdio-mediawiki
wdio-mediawiki v1.1.1:
- Includes wdio-defaults.conf.js file that vastly simplifies wdio.conf.js.
- Replaces @wdio/spec-reporter with @wdio/dot-reporter.
- Introduces video recording.

Bug: T283597
Change-Id: I603626d20eb060ba6d90b1a91d49afe13e29d945
2021-05-28 01:04:55 +05:30
jenkins-bot 7e8cf0d101 Merge "Selenium: update README.md file" 2021-05-18 15:45:15 +00:00
jenkins-bot 7d0e50a2cd Merge "Use FauxRequest::setUpload in AbuseFilterUploadTestTrait::doUpload" 2021-05-15 12:22:10 +00:00
sahil 96706a53fd Selenium: update README.md file
Bug: T282237
Change-Id: I93d7538bf191e42460e5d54532f6775a05dac661
2021-05-07 18:45:07 +05:30
Daimona Eaytoy 58ad3d1542 Replace deprecated User::getEffectiveGroups
Bug: T281824
Change-Id: I5487d143277a44742048668c920bbad57ebe6af1
2021-05-06 15:35:35 +02:00
libraryupgrader 06cdddc9d0 build: Updating composer dependencies
* mediawiki/mediawiki-codesniffer: 35.0.0 → 36.0.0
* php-parallel-lint/php-parallel-lint: 1.2.0 → 1.3.0

Change-Id: I92d6f6d6f817765df24f845103a489624f4290f2
2021-05-02 06:41:54 +00:00
Umherirrender 1fa7a83f60 Use static closures where safe to use
Created by I25a17fb22b6b669e817317a0f45051ae9c608208

Change-Id: I533690311ca559685de8a4bf123348c9bcfa5931
2021-04-30 20:55:35 +02:00
mainframe98 a32d483ef4 Don't use p class="success" for success messages
These are part of legacy styles and aren't provided by all skins.
Using Html::successbox abstracts the classes away.
Internally that uses div class="successbox" instead.

Bug: T280766
Change-Id: I0cca59e2f391510095c2c6fb187ace5e91fdde8b
2021-04-30 18:19:31 +00:00
jenkins-bot 0dc93136d6 Merge "Improve test coverage of API modules" 2021-04-18 16:03:25 +00:00
Matěj Suchánek a2ee8c41e2 Improve test coverage of API modules
Also solve one a TODO.

Change-Id: I61a38f3c741274f00ad0ad4789106a943daef222
2021-04-18 10:37:38 +02:00
jenkins-bot 5cd39a51fa Merge "Remove the old parser" 2021-04-17 15:21:54 +00:00
Daimona Eaytoy f67c2d5434 Remove deprecated $wgAbuseFilterCustomActionsHandlers
Extensions should now specify custom actions using the
AbuseFilterCustomActions hook.

Change-Id: Id21640d406b18c627eedff39d3f246cf21e042b3
2021-04-11 14:49:50 +00:00
Daimona Eaytoy f8438a4647 Remove the old parser
All methods were moved to the new parser. Tests and other pieces were
adjusted to expect just a single parser. There are still some TODOs
(remove AFPTransitionBase, remove $this->mCur), but these are left for
another commit.

Note that the new parser was not renamed: this is because the names are
wrong anyway (CachingParser is more of an Evaluator than a Parser, and
AFPTreeParser is the real parser, and should be renamed as well).

NOTE to reviewers: this patch looks quite big, but if you diff the old
parser with the new version of the CachingParser, you'll notice that the
diff is actually small, since everything was basically copied verbatim.

Bug: T239990
Change-Id: Ie914ef64c70503a201b4d2dec698ca2fa8e69b10
2021-04-09 13:23:07 +00:00
Umherirrender 32f7ae140e Use ::class for class name
This works also for non-existing classes,
because it is resolved on compile time

Change-Id: Ia3a1484c9c4f46a128c367ddd057c41dd560111d
2021-04-08 20:54:48 +02:00
Daimona Eaytoy 3e2153b86b Update userCanViewRev to use Authority
Change-Id: Ia10acf499ce33af03eeea45e34779a00e6628fe1
2021-04-07 13:55:10 +02:00
DannyS712 5d8ac68310 Convert AbuseFilterDBTest to pure unit tests
No integration needed, use a mock user.

Change-Id: I206d019aec626e6e4c16de10ecf30a29d5ab12e5
2021-04-06 16:28:35 +00:00
jenkins-bot e616e5b71c Merge "selenium: Remove ChromeDriver dependency" 2021-03-26 19:02:24 +00:00
daniel 65c5fd6b51 Construct UserIdentityValue without actorId
The actorId parameter to the UserIdentityValue constructor has been
deprecated.

Change-Id: I4a22e761276a9fefa15c7b1554a0d03980d0c663
Needed-By: I9925906d11e47efaec3c1f48d5cb3f9896a982c1
2021-03-26 11:00:56 +01:00
daniel e954ecfe19 Fix UserIdentityValue constructor call in test
Change-Id: If905e442444d319114e86ce78007bdc6dbc9c9f2
Needed-By: I9925906d11e47efaec3c1f48d5cb3f9896a982c1
2021-03-25 15:13:15 +01:00
jenkins-bot 92ffc1a7ed Merge "Don't directly instantiate PageEditStash" 2021-03-25 10:51:09 +00:00
Petr Pchelko e92d74ad51 Don't directly instantiate PageEditStash
Needed-By: I7164b914299441bd0f82e764252c8b5d30b45fbe
Change-Id: Iab96a35be8f50fdbc66194bd8956d98b5b6b0032
2021-03-24 21:03:46 -06:00
Željko Filipin 682b76662e selenium: Remove ChromeDriver dependency
Since WebdriverIO v5, Puppeteer is available as a Chromedriver
alternative.

Puppeteer is bundled with WebdriverIO. Chromedriver needs to be installed
(and started/stopped) separately. Getting rid of Chromedriver simplifies
our documentation, among other things.

The commit updates tests/selenium/wdio.conf.js to use Puppeteer.

Bug: T269566
Change-Id: Ib2a547792a34e6d40137432f7800b5f71c254c36
2021-03-23 18:03:29 +00:00
jenkins-bot fa8358ce0c Merge "Replace RecentChange::getPerformer with RecentChange::getPerformerIdentity" 2021-03-18 14:15:11 +00:00
Vadim Kovalenko 85be3c57bc Replace RecentChange::getPerformer with RecentChange::getPerformerIdentity
Bug: T276412
Change-Id: I8a55bd5cb17bbc259ec36c40261058e0b46ee4a6
2021-03-15 16:57:40 +02:00
jenkins-bot c0cb48abaf Merge "UserIdentityValue: do not provide actor ID" 2021-03-13 21:30:27 +00:00
jenkins-bot 1c5e5eb1e2 Merge "Create distinct builders for plain and ace editor" 2021-03-13 13:36:53 +00:00
daniel 05e416ef11 UserIdentityValue: do not provide actor ID
Depends-On: Ic2ca6d489db821fc2334e53bf2496c7b0d3ea5b1
Change-Id: I76e0a5b0dd68b763bdd0c5e4fe5d09f98e350ce1
Needed-By: I9925906d11e47efaec3c1f48d5cb3f9896a982c1
Bug: T274179
2021-03-12 16:38:51 +01:00
Daimona Eaytoy c5d19577a4 Fix method names of hook interfaces
The hook names contain a dash, which is mapped to an underscore by the
hook runner (see Ie8c8fb603b33ff95c8f8d52f392227f147c528d8), and the
previous method names weren't matching this.

Follow-up: Ic5c82a367e34135bbc0f00ece5aeef4f2d92881b

Change-Id: Ie80b62c49b2f4aaea49d5a1883f513348689d16a
2021-03-09 17:03:14 +00:00
Vadim Kovalenko 62376f437e Replace UserIdentity::getUserId with ::getId
Bug: T275482
Change-Id: Ie9fd52005ca1eb264dc791a2f87d7308a9e8810e
2021-03-08 09:18:49 -05:00
Daimona Eaytoy 6ba8e93537 Create distinct builders for plain and ace editor
Change-Id: I9d2b7572fed6e0b3660d3b0d5dad324d6b75fde9
2021-03-08 09:44:58 +00:00
jenkins-bot 12f4e81964 Merge "Simplify AbuseFilterBlockTest" 2021-03-06 09:23:26 +00:00