Commit graph

5061 commits

Author SHA1 Message Date
Daimona Eaytoy 393e47c5a7 Upgrade phan-config to 0.7.1
Change-Id: I859d81eda8601da91602b27a223b6d6d59ecf563
2019-09-01 09:42:26 +00:00
jenkins-bot 25d63aa639 Merge "Add new number syntax as experimental" 2019-08-31 17:12:10 +00:00
jenkins-bot 134c75dd5e Merge "Remove AbuseFilter::saveFilter dependency on AbuseFilterViewEdit" 2019-08-31 17:06:00 +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
Translation updater bot 3d98911e81 Localisation updates from https://translatewiki.net.
Change-Id: I3b99f9b4a9146cfb0f68dfc5b19e62a8efad8029
2019-08-31 15:55:18 +02:00
jenkins-bot fc3349df1f Merge "Fix param validation in ViewEdit" 2019-08-30 13:37:21 +00:00
Daimona Eaytoy 933b791ef3 Fix param validation in ViewEdit
We didn't check if the provided ID was valid. While editing an existing
filter (or creating a new one), we check the ID in SpecialAbuseFilter,
so it's guaranteed to get an integer in ViewEdit, and the case of a
non-existing filter is handled later, in buildFilterEditor.
But for links like Special:AbuseFilter/history/foobarbaz/item/1 (where
"foobarbaz" should be the filter ID), no validation was performed. This
caused a useless query to be carried out on the abuse_filter_history table (which would likely return false), then accessing properties of a non-object ('$row->afh_id'), and we ended up showing filter 1. This was spotted because we actually got notices in production.

Bug: T231632
Change-Id: I6436c7d2df8c1f0fc971f4a4079dac9118aa8209
2019-08-30 08:59:49 +00:00
Translation updater bot 131fc14d5b Localisation updates from https://translatewiki.net.
Change-Id: I32cc9bf7b9455c202da263e1ed210517e2ddab67
2019-08-30 10:00:51 +02:00
Daimona Eaytoy fdb71b5868 Make AbuseFilterVariableHolder::$mVars public again
+fixme comment per T231542#5451567.

Bug: T231542
Change-Id: If8b7b0568568df93548f12ccdc85fa174ec3558e
2019-08-29 18:51:43 +02:00
Daimona Eaytoy eb91595d8a Use row->afl_action instead of $vars
There's not need to use the variableholder in this hacky (albeit common)
whay, since the row already holds the action.
Note, this doesn't guarantee that the next two lines won't fail - I'd
need to see the actual var dump (T231542#5450720) to determine exactly
why this is failing.

Bug: T231542
Change-Id: I2112b046d00e06b575d15ab3d7da57484fd9cbbd
2019-08-29 13:48:31 +02: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
jenkins-bot c71874c9fa Merge "Refactor and speed up non-parser tests" 2019-08-28 15:25:58 +00:00
Translation updater bot fb1864dfa5 Localisation updates from https://translatewiki.net.
Change-Id: I18c2528207d8f02842ea68f1ef9e15a0a555ce12
2019-08-28 10:51:00 +02:00
Daimona Eaytoy 540edc5174 Remove AbuseFilter::saveFilter dependency on AbuseFilterViewEdit
This dependency is wrong, and removing it will also allow creating an
edit API.

Bug: T213037
Depends-On: Id8412e2b8a4e873fd4821ecc1a3c95710be9a870
Change-Id: If8e745a3227cea5093ea3fd8f5b201adedaba3ae
2019-08-27 16:26:18 +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 c469fb4b76 Mostly remove $wgUser
There are lots of cases where we can inject a User object without
additional efforts. Now $wgUser is only used inside AFComputedVariable,
which is a little bit harder to handle because some instances of that
class are serialized in the DB, and thus we cannot easily change the
constructor until T213006 is resolved.

This partly copies what Ia474f02dfeee8c7d067ee7e555c08cbfef08f6a6 tried
to do, but adopting a different approach for various can*() methods:
they're now static methods in the AbuseFilter class, so future callers
don't need to instantiate an AbuseFilterView class. This also allows to
re-use those methods in an API module for editing filters (T213037).

Bug: T213037
Bug: T159299
Change-Id: I22743557e162fd23b3b4e52951a649d8c21109c8
2019-08-27 13:20:37 +02: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
Daimona Eaytoy 4d86758a49 Add new number syntax as experimental
For now it will only report successful parse. Next step is formally
deprecating the old one (escalated to warning), then removing it in
favour of the new one (in another MW version).

Bug: T212730
Change-Id: I5dd11fd67d8e57d1d0c52ddfa026920ebfc5ee13
2019-08-26 08:15:55 +00:00
Translation updater bot e96d5ef301 Localisation updates from https://translatewiki.net.
Change-Id: I0f95254a1d35d82df1c758809571d4a053d25b37
2019-08-26 09:05:28 +02:00
jenkins-bot 89524790d5 Merge "Add a hook to determine whether the current action should be filtered" 2019-08-25 18:45:07 +00: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
Translation updater bot d486489ab9 Localisation updates from https://translatewiki.net.
Change-Id: I79b1f163791247a8158270e9bc2da771a42d5989
2019-08-25 10:28:34 +02: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
Daimona Eaytoy 7f554734e6 Don't hardcode blockautopromote duration
As explained on phab, and add a script to fix broken entries.

Bug: T231131
Change-Id: I95d70acb936b5ca987af8f237d236fe47b663919
2019-08-24 11:40:11 +02:00
libraryupgrader 6b5d20c9b7 build: Updating eslint-utils to 1.4.2
* https://npmjs.com/advisories/1118

Additional changes:
* Added .eslintcache to .gitignore.

Change-Id: I13f3123608242ac1bee91ee05130926375d3f485
2019-08-24 04:08:38 +00:00
Huji Lee 1ddb65021b Add links to AbuseFilter logs on Special:Undelete
Depends-On: I671a0479e877e6c37606b688064cb9c893717709
Bug: T231055
Change-Id: Iebf832c513c6a4e954db0ba2633dd8ba6f27b412
2019-08-23 14:56:43 +00:00
Daimona Eaytoy bf61414f88 Don't show empty "Tools:" section in ViewEdit
After having removed the export link in
I72f46247f4323fb5bfe7fa74f332076dbd346187, we don't have any tool to
show for new filters. So avoid outputting an empty section.

Change-Id: Ia07bccdbadb7b874397135bc3f7468d6e0b9eb13
2019-08-21 17:32:43 +02:00
Translation updater bot 38676f0ac7 Localisation updates from https://translatewiki.net.
Change-Id: I0651cb3ed1c15cadb49d85e580afc3b1fdb2162e
2019-08-21 09:59:07 +02: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
jenkins-bot a8e2071351 Merge "Better handling of function params in CachingParser" 2019-08-20 18:46:01 +00:00
jenkins-bot 8527a10774 Merge "Restyle edit box dimensions" 2019-08-20 16:33:16 +00: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 7addec7b4a Merge "Make some other AFPData methods non-static" 2019-08-20 14:16:16 +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
jenkins-bot f1ab591d27 Merge "Avoid implicit casts from DUNDEFINED to something else" 2019-08-20 13:04:48 +00:00
jenkins-bot ea01809f5e Merge "Add the filter ID to empty operand logging" 2019-08-20 13:01:14 +00:00
jenkins-bot d32b03ca10 Merge "Increase cache hits for CachingParser" 2019-08-20 12:50:31 +00:00
jenkins-bot d0b30c2534 Merge "Make parser aware of the filter it is parsing" 2019-08-20 12:50:26 +00:00
jenkins-bot 1bfd182a2e Merge "Fix object to array cast, typehint array params" 2019-08-20 12:49:09 +00:00
jenkins-bot 671b8b7fb3 Merge "Distinguish AbuseFilter warnings and errors in VE" 2019-08-20 12:32:26 +00:00
Translation updater bot 6bcc2d2ccd Localisation updates from https://translatewiki.net.
Change-Id: I90f7c5a69e7d615a7d30eb2440f8d65635cfb2c0
2019-08-20 10:16:41 +02:00