This is mostly about adding return types to methods that implement PHP
interfaces, and not passing null to core functions that want a string.
After this patch, and an update to return types in RemexHtml,
tests/phpunit/integration/ has no more errors than in PHP 8.0.
Bug: T289879
Bug: T289926
Change-Id: Ia424f5cc897070f4188ae126b5bf6a1f552db0e1
This reverts commit ec3da4589bebeb46d7f1544dc46f24baec334966.
Caused frequent session loss in the Wikimedia cluster.
Bug: T299193
Bug: T309616
Change-Id: I3a410df88071d72078672cf1b670e81c11b28117
(cherry picked from commit d1a1fcedc9eace8a5f4a8454eff44a7ed898848a)
SessionBackend::save() results in the session provider unpersisting
when the session backend metadata is dirty (which is always true
for a new session). This breaks the tombstone mechanism introduced
in I3a76b67aa51159ebf0195db15cf7c34e00a64a2e: after the manager
refuses to load the tombstoned session, it will create an empty
session, and that will unpersist the session ID and log the user
out if the session ID was the only thing that kept them logged in.
Saving the session has two effects: the unpersisting (invalid
cookies are cleaned up) and saving the session data in in-process
cache (but not the real storage). The latter might cause an extra
session storage read per unauthenticated request, though in theory
it shouldn't as the SessionBackend itself gets cached so a new
session lookup only happens for a different WebRequest object.
Still, if it becomes a problem, we can just add some sort of
explicit cache warming step to empty session creation.
Skipping the unpersisting could mean that invalid (e.g. expired)
session cookies stick around and prevent the user from being
served from edge cache. But for non-tombstoned requests, as long
as there is a syntactically valid session ID and either no user
ID or a user ID pointing to a valid user, CookieSessionProvider
will return a non-null session info, and SessionManager will
reject it and unpersist; and CentralAuthSessionProvider behaves
similarly except for the case when there is no local session
cookie but there is a global session cookie on the second-level
domain. That seems a narrow enough edge case to ignore.
Bug: T299193
Change-Id: Ib34a84d1d3abbce4dcf7433b51abf6e694984c59
The getConfig of a ContextSource should only be used, if the
ContextSource is available. Getting the global context just for the
config looks harder to fix/inject as using the MainConfig from
MediaWikiServices
Change-Id: Iaf14bfc7bd68cc315672e1c256887faf87e22542
SessionBackend::resetId() is prone to race conditions with
cookie-based session providers, where MediaWiki receives
a request with the old session and forces the client to log
out. To handle that, add a tombstone mechanism to
SessionBackend, so instead of deleting the old session from
the store on ID reset, it is marked as invalid. Tombstoned
sessions are handled as nonexistent ones, except unpersist()
is not called.
Unlike Iffd69c7f246adff40b07668328a07329440dbd6f this doesn't
prevent overwriting the session if the MediaWiki endpoint calls
persist() or unpersist(), but it is vastly simpler, and very
few endpoints persist the session.
The behavior of SessionManager::loadSessionInfoFromStore()
with a tombstoned session and SessionInfo::forceUse()===true
does not make much sense, but that's a nonsensical scenario
in the first place (it only happens when the session provider
returns true from persistsSessionId() but sets the forceUse
flag which is meant for providers which can't change the
session ID) and we are only really concerned here about
cookie-based sessions anyway.
Bug: T299193
Change-Id: I3a76b67aa51159ebf0195db15cf7c34e00a64a2e
SessionManager is a singleton and thus this could lead to
storing an outdated ObjectFactory instance.
Bug: T307998
Change-Id: I5bacb45cc0d85c21907e22bc9bbb32f6286b8cc5
Now largely automated:
VARS=$(grep -o "'[A-Za-z0-9_]*'" includes/MainConfigNames.php | \
tr "\n" '|' | sed "s/|$/\n/;s/'//g")
sed -i -E "s/'($VARS)'/MainConfigNames::\1/g" \
$(grep -ERIl "'($VARS)'" includes/)
Then git add -p with lots of error-prone manual checking. Then
semi-manually add all the necessary "use" lines:
vim $(grep -L 'use MediaWiki\\MainConfigNames;' \
$(git diff --cached --name-only --diff-filter=M HEAD^))
I didn't bother fixing lines that were over 100 characters unless they
were over 120 and triggered phpcs.
Bug: T305805
Change-Id: I74e0ab511abecb276717ad4276a124760a268147
* In PermissionManager, if a user is anonymous but temporary user
creation is possible, grant elevated permissions at RIGOR_QUICK rigor
level. This is mostly to make skins show "edit" instead of "view
source" to anonymous users in the recommended permissions
configuration.
* Present temporary users as if they are not logged in in various places
in the interface: create/move permissions errors, login, preferences,
watchlist, BotPasswords, ChangeEmail, ResetTokens.
* Show a warning on login/logout about loss of access to the temp
account.
* On login, don't show the temporary name as a suggestion for the login
username.
Change-Id: Id0d5ffa46c3ca5c7b30d540cedbaa528b682aa85
This covers all occurrences of /onfig->.*get( '/ in includes/.
Undoubtedly there are still plenty more to go.
Change-Id: I33196c4153437778496f40436bcde399638ac361
MWGrants is deprecated and should be replaced with the GrantsInfo and
the GrantsLocalization services.
Bug: T253077
Change-Id: I3cbf568b6de654acb6b06b4ab5d9d97a09f78ece
These two interfaces' methods have tentative return types in PHP 8.1,
which causes code without the type hints to raise warnings. Where the
type hint is "mixed", we need to use the special declaration
[\ReturnTypeWillChange] in a comment to suppress the warning as long as
we still support PHP < 8.0, which doesn't have a "mixed" type hint.
Bug: T289879
Change-Id: I1a126e602e92b8d13c7795eb6d790effd5ddc986
Use name constants instead of string literals in calls to Config::get
and ServiceOptions::get, when referring to core configuration variables.
This protects against typos and makes the decumentation and schema
declaration of the config settings discoverable.
This is the first batch, only touching files directly under /includes/
Change-Id: I7252e636c7c86d950d9257b33491af492c6dd5eb
Make phan stricter about conditional variable declaration
Remaining false positive issues are suppressed.
The suppression and the setting change can only be done together
Bug: T259172
Change-Id: I1f200ac37df7448453688bf464a8250c97313e5d
Make phan stricter about array keys
Remaining false positive issues are suppressed.
The suppression and the setting change can only be done together
Bug: T304887
Depends-On: I3105a5fd4826f8667b5232834defc5ec93be32a1
Depends-On: Ie9610a6e83731468311edb3ed17f80fc509de385
Change-Id: I701f12ab94478c3b8e7fd82110ade74a8e6b04ef
Conventionally, public constants are accessed via their declaring
class, except for self:: which is an acceptable shortcut.
Change-Id: If05eab72140267e6ef54736710d751d7f24a7860
Make phan stricter about null types by setting null_casts_as_any_type to
false (the default in mediawiki-phan-config)
Remaining false positive issues are suppressed.
The suppression and the setting change can only be done together
Bug: T242536
Bug: T301991
Change-Id: I0f295382b96fb3be8037a01c10487d9d591e7e01
Make phan stricter about scalar types by setting scalar_implicit_cast to
false (the default in mediawiki-phan-config)
Bug: T242536
Bug: T301991
Change-Id: Ia2fe30b17804186571722e728578121c8b75d455
* Some functions accept only string, cast ints and floats to string
* After preg_matches or explode() casts numbers to int to do maths
* Cast unix timestamps to int to do maths
* Cast return values from timestamp format function to int
* Cast bitwise operator to bool when needed as bool
* php internal functions like floor/round/ceil documented to return
float, most cases the result is used as int, added casts
Found by phan strict checks
Change-Id: Icb2de32107f43817acc45fe296fb77acf65c1786
ObjectFactory needs a service container from which it can fetch the
services to be injected. ObjectFactory::createObject automatically
uses the one being injected into ObjectFactory.
This now allows easy injection of services into session providers.
Change-Id: I8e33e7ff08cef039104b967ce44f29ef106d0a4d
When the called function has a doc of int, it should be cast to be
explicit here.
Also cast for arithmetic operations to be explicit about the number
Change-Id: I905b78dfb66e66443e0e3203488bab5b548db543
… including PHPDoc tags like `@return <type> $variableName`.
A return value doesn't have a variable name. I can see that
some people do this intentionally, repeating the variable
name that was used in the final `return $var;` at the end
of a method. This can indeed be helpful. I leave a lot of
these untouched and removed them only when it's obviously
wrong, or does not provide any additional information in
addition to what the code already says.
Change-Id: Ia18cd9f25ef658b08ad25b97a744897e2a8deffc
The documentation of SessionProvider::init says that the method can be used in implementation tests.
Now all tests must use SessionProviderTestTrait::initProvider in order to initialize a SessionProvider.
So the entire description after @internal was rewritten. Also the
similar note was added to the documentation of
AbstractAuthenticationProvider::init.
Bug: T283931
Change-Id: I1cde6fd12666ef3a7c9b1e33a718592c11cf9e29
This reverts commit ecf826a2ee.
Reason for revert: need to edit the patch and then it will be GTG in order to finish hard deprecating of User ::getCanonicalName, ::isUsableName, ::isCreatableName
Change-Id: I2f57f56728fcbeada96dc2228f07dc8bcaa5d4f6
In all these cases the property is unconditionally set in
the constructor. The extra initialisation is effectively
dead code and an extra source of errors and confusion.
Change-Id: Icae13390d5ca5c14e2754f3be4eb956dd7f54ac4
init() method was added to SessionProvider. It helps to inject
dependecies. Overridable postInitSetup() method was added to
SessionProvider. A provider can override this to do
any necessary setup.
SessionProvider ::setLogger(), ::setManager(), ::setConfig(),
::setHookContainer() were soft deprecated. Now developers must use
SessionProvider::init()
Bug: T275030
Change-Id: I958797881251572e4ec22e7dd9d13f74f7e216a3
