Exceptions classes are nearly always value objects, and should in most
cases by newable.
Bug: T247862
Change-Id: I4faa8ec6ea8bc44086cfc8075b32d10eea61e9df
This annotates classes that can safely be instantiated by
extensions, per the Stable Interface Policy.
Bug: T247862
Change-Id: Ia280f559874fc0750265ddeb7f831e65fd7d7d6a
* Add $wgCookieSameSite, which controls the SameSite attribute for login
cookies. This will need to be set to "None" on WMF and other wikis
with a CentralAuth installation spanning multiple registrable domains.
* Add $wgUseSameSiteLegacyCookies, which causes a "legacy" cookie to be
sent without a SameSite attribute whenever a SameSite=None cookie is
sent. I used the prefix "ss0" since it's like SameSite version 0, and
that's shorter than "legacy". It's a prefix instead of a suffix to
avoid the need to update the VCL config which identifies cookie types
by their name suffix.
* Simplify WebRequest::getCookie() removing the unnecessary unicode
normalization. This was added by analogy with GET/POST, I don't
believe it was ever necessary for cookies.
* Add WebRequest::getCrossSiteCookie(), which implements the read side
of the legacy SameSite cookie support.
* Fix Doxygen formatting of the parameter list in
WebResponse::setCookie().
* To work around the lack of SameSite cookie support in PHP 7.2, emulate
setcookie() with header() where necessary.
Bug: T252236
Change-Id: I141ea114fea007a72a4f24bfc34dd81100854d68
Add $wgForceHTTPS. When set to true:
* It makes the HTTP to HTTPS redirect unconditional and suppresses the
forceHTTPS cookie.
* It makes session cookies be secure.
* In the Action API, it triggers the existing deprecation warning and
avoids more expensive user/session checks.
* In login and signup, it suppresses the old hidden form fields for
protocol switching.
* It hides the prefershttps user preference.
Other changes:
* Factor out the HTTPS redirect in MediaWiki::main() into
maybeDoHttpsRedirect() and shouldDoHttpRedirect(). Improve
documentation.
* User::requiresHTTPS() reflects $wgForceHTTPS whereas the Session
concept of "force HTTPS" does not. The documentation of
User::requiresHTTPS() says that it includes configuration, and
retaining this definition was beneficial for some callers. Whereas
Session::shouldForceHTTPS() was used fairly narrowly as the value
of the forceHTTPS cookie, and injecting configuration into it is not
so easy or beneficial, so I left it as it was, except for clarifying
the documentation.
* Deprecate the following hooks: BeforeHttpsRedirect, UserRequiresHTTPS,
CanIPUseHTTPS. No known extension uses them, and they're not compatible
with the long-term goal of ending support for mixed-protocol wikis.
BeforeHttpsRedirect was documented as unstable from its inception.
CanIPUseHTTPS was a WMF config hack now superseded by GFOC's SNI
sniffing.
* For tests which failed with $wgForceHTTPS=true, I mostly split the
tests, testing each configuration value separately.
* Add ArrayUtils::cartesianProduct() as a helper for generating
combinations of boolean options in the session tests.
Bug: T256095
Change-Id: Iefb5ba55af35350dfc7c050f9fb8f4e8a79751cb
This patch replaces all usages of @protected in core.
The @protected tag was removed in cases where it was redundant or
contradictory. It has been replaced by @internal where usage outside of
core is not desired, and with @note for cases where use by extensions
is desired, but should be limited.
Bug: T247862
Change-Id: I5da208e5cb4504dde4113afb3a44922fd01325a3
Migrate all callers of Hooks::run() to use the new
HookContainer/HookRunner system.
General principles:
* Use DI if it is already used. We're not changing the way state is
managed in this patch.
* HookContainer is always injected, not HookRunner. HookContainer
is a service, it's a more generic interface, it is the only
thing that provides isRegistered() which is needed in some cases,
and a HookRunner can be efficiently constructed from it
(confirmed by benchmark). Because HookContainer is needed
for object construction, it is also needed by all factories.
* "Ask your friendly local base class". Big hierarchies like
SpecialPage and ApiBase have getHookContainer() and getHookRunner()
methods in the base class, and classes that extend that base class
are not expected to know or care where the base class gets its
HookContainer from.
* ProtectedHookAccessorTrait provides protected getHookContainer() and
getHookRunner() methods, getting them from the global service
container. The point of this is to ease migration to DI by ensuring
that call sites ask their local friendly base class rather than
getting a HookRunner from the service container directly.
* Private $this->hookRunner. In some smaller classes where accessor
methods did not seem warranted, there is a private HookRunner property
which is accessed directly. Very rarely (two cases), there is a
protected property, for consistency with code that conventionally
assumes protected=private, but in cases where the class might actually
be overridden, a protected accessor is preferred over a protected
property.
* The last resort: Hooks::runner(). Mostly for static, file-scope and
global code. In a few cases it was used for objects with broken
construction schemes, out of horror or laziness.
Constructors with new required arguments:
* AuthManager
* BadFileLookup
* BlockManager
* ClassicInterwikiLookup
* ContentHandlerFactory
* ContentSecurityPolicy
* DefaultOptionsManager
* DerivedPageDataUpdater
* FullSearchResultWidget
* HtmlCacheUpdater
* LanguageFactory
* LanguageNameUtils
* LinkRenderer
* LinkRendererFactory
* LocalisationCache
* MagicWordFactory
* MessageCache
* NamespaceInfo
* PageEditStash
* PageHandlerFactory
* PageUpdater
* ParserFactory
* PermissionManager
* RevisionStore
* RevisionStoreFactory
* SearchEngineConfig
* SearchEngineFactory
* SearchFormWidget
* SearchNearMatcher
* SessionBackend
* SpecialPageFactory
* UserNameUtils
* UserOptionsManager
* WatchedItemQueryService
* WatchedItemStore
Constructors with new optional arguments:
* DefaultPreferencesFactory
* Language
* LinkHolderArray
* MovePage
* Parser
* ParserCache
* PasswordReset
* Router
setHookContainer() now required after construction:
* AuthenticationProvider
* ResourceLoaderModule
* SearchEngine
Change-Id: Id442b0dbe43aba84bd5cf801d86dedc768b082c7
This was deprecated in hooks.txt as deprecated in 1.27 but never actually
hard-deprecated.
Bug: T232880
Change-Id: I2195f672a44ae107937c61718f9ef19073db169f
Add hook interfaces which were generated by a script which parses
hooks.txt and identifies caller namespaces and directories.
Hook interfaces are mostly placed in a Hook/ subdirectory
relative to the caller location. When there are callers in multiple
directories, a "primary" caller was manually selected. The exceptions to
this are:
* The source root, maintenance and tests, which use includes/Hook. Test
hooks need to be autoloadable in a non-test request so that
implementing test interfaces in a generic handler will not fail.
* resources uses includes/resourceloader/Hook
* The following third-level subdirectories had their hooks placed in
the parent ../Hook:
* includes/filerepo/file
* includes/search/searchwidgets
* includes/specials/forms
* includes/specials/helpers
* includes/specials/pagers
Parameters marked as legacy references in hooks.txt are passed
by value in the interfaces.
Bug: T240307
Change-Id: I6efe2e7dd1f0c6a3d0f4d100a4c34e41f8428720
CSRF tokens should only be required (and only be allowed) if
the current session isn't already inherently safe against
CSRF due to the way the authentication mechanism works.
This allows (and requires) tokens to be omitted for requests
that use an OAuth Authorization header.
Bug: T230843
Bug: T230842
Bug: T237852
Change-Id: Ib2922d556ff2470d4bf8c386c18986ca9f37d1b5
Most SessionProviderInterface implementations require CSRF protection,
but some (notably MWOAuthSessionProvider from the OAuth extension)
do not. Add a function for the implementing class to indicate whether
or not the provider is safe against CSRF protection.
Bug: T237852
Change-Id: Ib452b6c75aa7d40dd211a6064f97509b664c3ffc
In all these cases, the foreach() loop specifies a variable for the
current value. We don't need two ways to access the same value. This
makes the code harder to read.
Change-Id: I6ed7a518439963b7091057194de993a7e977be32
Scalar casts are still allowed (for now), because there's a huge amount
of false positives. Ditto for invalid array offsets.
Thoughts about the rest: luckily, many false positives with array offsets
have gone. Moreover, since *Internal issues are suppressed in the base
config, we can remove inline suppressions.
Unfortunately, there are a couple of new issues about array additions
with only false positives, because apparently they don't take
branches into account.
Change-Id: I5a3913c6e762f77bfdae55051a395fae95d1f841
Repeating the variable name doesn't do anything. Documentation
generators don't need it. It's more stuff to read that doesn't add new
information. And it can become outdated.
Note there are two types of @var docs. When used inline (and not on a
class property) the variable name is needed.
Change-Id: If5a520405efacd8cefd90b878c999b842b91ac61
This is overridden in CentralAuth, so I54a152d7b1b16fdc892a54563e73d63af314a169
should be merged straight after this one.
Change-Id: I3b253f6e06aa496b2dd6ef891caebddc22b4f035
These were all checked with codesearch to ensure nothing is overriding
these methods.
For the most part, I've updated the signature to use nullable types; for
two Pager's, I've just made all parameters non-optional, because you're
already forced to pass them with a required parameter at the end.
Bug: T231636
Change-Id: Ie047891f55fcd322039194cfa9a8549e4f1f6f14
This allows us to remove many suppressions for phan false positives.
Bug: T231636
Depends-On: I82a279e1f7b0fdefd3bb712e46c7d0665429d065
Change-Id: I5c251e9584a1ae9fb1577afcafb5001e0dcd41c7
All of these suppression prevent the detection of many common mistakes,
and could easily prevent things like T231488. Especially if there are
few issues of a given type, it's way better to suppress them inline,
instead of disabling them for the whole core.
This patch only touches the one with a lower count (although those
counts may be out of date).
Bug: T231636
Change-Id: Ica50297ec7c71a81ba2204f9763499da925067bd
SessionManager::singleton is returning the interface and that is used
for PHPSessionHandler::install, so switching the whole class to the
interface as type hint
Change-Id: I400ad1d13df92983142e735749ee8d2e12d28b34
Clean up a few more code paths and documentation bits left behind by
Ia53d07cd8ce8ab1497294ea244c13c7499f632c7.
Change-Id: I2bb1749c45bb79b27c5a3b2e1b8ed3395e8c11e0
These implemented a since-abandoned draft IETF spec, and the code was
broken due to (1) case-(in)sensitivity issues with the Accept-Language
header and (2) the BCP47 language code compatibility workaround we use.
Change-Id: Ia53d07cd8ce8ab1497294ea244c13c7499f632c7
Wikimedia\quietCall() is deprecated and AtEase is here for use.
I would have loved to do restoreWarnings() and suppressWarnings()
in this same patch set but will continue the work for a later patch.
Bug: T182273
Change-Id: I43e3a5f378c99b5c40883b35ba133cbd126fc433
I benchmarked this again. The runtime of an unlimited explode() can be
quite high. This is not really a DoS attack vector as it would require to
post megabytes worth of input to the code, which will hit many other
limits before. I still consider it good practice to use unlimited explode()
only when it is actually allowed to return an unlimited amount of elements.
Change-Id: I30f8ca5dba7b317bb4a046b9740fd736b4eea291
