After temporary accounts are enabled, filters that rely on an ip
in the `user_name` will fail (eg. `ip_in_range` and `ip_in_ranges`).
To keep these filters working:
- Expose the IP through another variable, `user_unnamed_ip`, that can be
used instead of `user_name`.
- The variable is scoped to only reveal the IPs of temporary accounts
and un-logged in users.
- Wikis that don't have temporary accounts enabled will be able to see
this variable but it won't provide information that `user_name`
wasn't already providing
- Introduce the concept of transforming variable values before writing
to the blob store and after retrieval, as IPs need to be deleted from
the logs eventually and can't be stored as-is in the amend-only blob
store
Bug: T357772
Change-Id: I8c11e06ccb9e78b9a991e033fe43f5dded8f7bb2
Instead of having separate methods for each variable,
have one method which can work not only with "_links",
but with any array of strings.
Change-Id: I05f1b1cbd15f283b314c72259f183f7788e4e214
The following sniffs are failing and were disabled:
* MediaWiki.Commenting.FunctionComment.MissingDocumentationPublic
Change-Id: I6075c76d53a899aac56af027f9a956a6b9e6a667
Why:
* An AbuseFilter variable is needed that allows filters to determine
what type the current user is. That is, whether the user is an
IP address, temporary account, named user or external user.
* Currently filters implement this by inspecting the value in
the 'user_name' variable, but this is likely to break when
temporary accounts are enabled as IPs would be hidden.
* Giving a dedicated variable that indicates the type of the user
allows filters to work out this information without having to
know the specific username of the user before performing the
check.
What:
* Add the 'user_type' variable which is lazily computed. It can have
the value 'named', 'temp', 'ip' or 'external' depending on the
type of the user. If the user does not match any of these, then
the value is 'unknown'.
* Replace call to deprecated User::newFromIdentity with a use of the
UserFactory service that is dependency injected.
* Add and update tests to ensure consistent test coverage.
Bug: T357615
Change-Id: Ifffa891879e7e49d2430a0330116b34c5a03049d
This requires 1.42 for some new names
Changes to the use statements done automatically via script
Addition of missing use statements and changes to docs done manually
Change-Id: Ic1e2c9a0c891382744e4792bba1effece48e53f3
This method actually consists of two: add derived vars, and initialize
content vars. The former part depends on no parameters of this method.
On the other hand, the latter part combines multiple implementations
for some of the content variables using branching.
The branching is a dirty workaround and inferior to the GRASP principle:
"When related alternatives or behaviors vary by type, assign
responsibility for the behavior to the types for which the behavior
varies."
In other words, the callers (extensions) should be responsible for
choosing the initialization strategy themselves, instead of letting
VariableGenerator figure it out.
As the first step, split the former part to a separate method.
For now, it will be implicitly called by ::addEditVars.
Change-Id: I5ff00dbdbf29ec54eabfd95c44a4fd7f713969f5
Protected effectively means "public to subclasses" and should be
avoided for the same reasons as marking everything as public should
be avoided.
Change-Id: Iba674b486ce53fd1f94f70163d47824e969abb77
Abuse filter needs to check both if the update is available and if the
page is rendered. This is the exact issue FlaggedRevs have:
050b9593fb/backend/FlaggedRevs.php (L718)
Bug: T339094
Change-Id: I943c8dbb525dc4c988e97e180474ea71b4cf731d
When forFilter is true and PreparedUpdate is available
(most save operations), retrieve all_links from
PreparedUpdate::getParserOutputForMetaData. Otherwise
do what was done before.
Note that this change probably leaves some dead code. It will be dealt
with later.
NOTE: this changes code potentially executed on every save operation.
Bug: T65632
Bug: T264104
Change-Id: I3628a56e5277846c1b90444fb55983870eb54c1e
The method for old_links retrieval depends on the "forFilter"
value, which we know in advance. If it's true, old_links should
be retrieved from the database. Make a case in the switch
that does nothing but retrieves links from the database,
and direct the evaluation to it.
This change was split from I3628a56e5 to make its review easier.
NOTE: this changes code potentially executed on every save operation.
Change-Id: I33b688f6be3c58beec403f7bf26407a42e7c18ab
We might consider adding an in-process cache because there
will be a duplicate database lookup for content model and
wikitext of the same revision.
Bug: T230295
Change-Id: I9723f21069e03a49fa7131bd8f79c6e7e442104b
Each generator knows in which situation it is executed, and it
can pass this information to the computer. VariableHolder should
just hold the variables.
Change-Id: I0fb2e01e3e9457cd63948afe2a20439a1c800790
These are apparently the only two variables for which we can
quickly determine their value in such simple way.
Later, we can also try it for recent contributions.
Bug: T102944
Change-Id: Iecfa9e5c5ba8c078691334b676cc6f289790cb74
This was most definitely my intention when I introduced the concept of
"generic vars", so it's a bit surprising to discover, 3.5 years later,
that the timestamp isn't computed there.
Also make the timestamp always be a string for consistency, since that's
the type documented on mw.org. I've manually checked all filters on
Wikimedia wikis using the timestamp variable, and added explicit int
casts where needed (although I think they'd still work due to implicit
casts).
Change-Id: Ib6e15225dd95c2eead7e48c200d203d6918e0c18
The check was not consistent and the code could still crash
when $oldContent was null. RevisionRecord:getContent only
returns null when audience check fails, but we don't ask
for that.
Change-Id: Id64646a6762167f552e104f623130bedc6b2dd18
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
Previously, for non-newly-created pages, AbuseFilter would get the text
for filtering twice: once in AbuseFilterHooks::filterEdit(), and then
again in RunVariableGenerator::getEditTextForFiltering(). (Plus another
call for the text of the previous revision.) The first copy of the text
is only passed into RunVariableGenerator::getEditVars(), and there only
used if the title doesn’t exist, otherwise it’s overwritten with the
second copy. Instead, let’s make AbuseFilterHooks not get the text at
all, and only get the text from the content when we actually need it
(the content is new).
Change-Id: Id12430fa6ba4643113b945e0d0c01b9c0ee1742f
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
With explicit calls it's easier to see what method is being used,
whether it's deprecated, etc. Some methods here are in fact deprecated
or already have a proper replacement, but this is left for a follow-up.
Change-Id: Iee3154855f86c76aab98e7c14250c14e8b9ee939
Additionally, avoid building Title objects in LazyVariableComputer, it
just adds a dependency on TitleFactory and creating mocks is more
complicated, but it's pointless because the caller already has a Title
object.
And also stop using Title::getEarliestRevTime(), since the replacement
is easy (we already have a RevisionLookup).
Note for reviewers about renames:
- Code VariableGeneratorDBTest was moved to LazyVariableComputerDBTest,
RCVariableGeneratorTest, and AbuseFilterVariableGeneratorTest
- AbuseFilterVariableGenerator test was moved into a dedicated
directory, methods were changed not to test the var values
Change-Id: I3dff8739a9b79f33321d836449b082c3ce63f277
So we can use DI in all generators. Some improvements were deliberately
omitted, e.g. injecting more services and relaxing User/Title to
UserIdentity/LinkTarget, and they'll be included in a subsequent commit.
Depends-On: I1f351071ef2b0b7c80e91407a9c3bb17be293044
Depends-On: Ie71740fac35a86f8fe03023080ae8ca08671243d
Depends-On: I589a0e1c2c5891070ab82cd5adfd9cedec19e67d
Change-Id: I92ef0abd5e45b672e6f297a71b3c2c345d56f136
This is an important step towards removing the AbuseFilter class. Note:
proposals for the name of the new service are welcome.
Change-Id: Ib4632173f728b1bdafadef96e01645a833bfceaa
See task for a description of the plan. Also note that
AFComputedVariable should be renamed and its properties made private.
This commit includes some adjustments for taint-check in
AbuseFilter::buildVarDumpTable and ::revisionToString.
There's some space for improvement in the new LazyVariableComputer, but
that's left for another commit.
Bug: T261069
Change-Id: Ia44f6e079d39f44cf0122dec5ddb5513ab54f0c6
This is the last use, and it was a bit harder to remove because it was
buried inside AFComputedVariable. Starting with
I4444cada720ab62d187f2dd0c4760697e465f2ff, we can freely change the
parameters to AFComputedVariable without breaking old log entries.
Note, we still need a fallback for other extensions calling this
method...
Bug: T246733
Depends-On: I4444cada720ab62d187f2dd0c4760697e465f2ff
Change-Id: I5d786a518ef88fad9c8d9c25ef4553a0bf30b2b2
