This is micro-optimization of closure code to avoid binding the closure
to $this where it is not needed.
Created by I25a17fb22b6b669e817317a0f45051ae9c608208
Change-Id: I0ffc6200f6c6693d78a3151cb8cea7dce7c21653
This includes fixing some mistakes, as well as removing
redundant text that doesn't add new information, either because
it literally repeats what the code already says, or is actually
duplicated.
Change-Id: I3a8dd8ce57192deda8916cc444c87d7ab1a36515
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
Updated Doxygen markup in several .php files triggering warnings when mwdocgen.php is executed. Removed
obsolete settings MSCGEN_PATH and TCL_SUBST from Doxyfile. The former would generate a warning in 1.8.16
while TCL support was removed in 1.8.18. Since TCL_SUBST was blank anyway, it was removed prior to getting
to .18 in production. Increased DOT_GRAPH_MAX_NODES from 50 to 200 since Doxygen complained about it being
too low for API and Maintenance.
Bug: T248706
Change-Id: I9c67f0807d1b43089d351263d4f591dee5501f36
The basic idea is: The sequence `$var ?? 'default'` either uses the
value from the variable (or array element) if available, or falls back
to the default value. The resulting value is then used in the if()
condition.
if ( $var ?? true ) means the variable should default to true, if not
set.
This is mostly a style change.
In ApiEditPage the $params are impossible to not be set.
Change-Id: Id67b81744fa21fe22a2d2377259e426aab67c479
blob: is basically a fancy version of eval(), commonly used with
web workers. We currently allow eval() ('unsafe-eval'), so we
might as well include blob. If we try and lock-down eval() at
some later point, we can re-evaluate this decision.
AFAIK, blob: urls are currently used with web workers in
NavigationTiming and CodeEditor extensions.
Bug: T245981
Change-Id: I3c44286e140ddfe2d3abd31428e0770ff5531e37
<object> and <embed> are from a mostly bygone era. They often can
be used to evade CSP rules, and are often a soft spot for browser
security.
The default value of 'none', disables <object>, <embed>. In some
browsers this will also disable loading some file formats like
pdf directly in an iframe.
The only use I am aware of is in TimedMediaHandler. However, it seems
like the mw.EmbedPlayerGeneric, mw.EmbedPlayerKplayer, and
mw.EmbedPlayerVlc.js are no longer used.
Bug: T239051
Change-Id: Iae7ab1f5b7c422803782848c787bc1a4c6339913
Adds method to the CSP object to add extra sources. This is
meant so, for example, a recaptcha extension could add the
recaptcha source on pages with captchas, or CodeEditor could
add blob: as allowed JS sources on pages that need that (T214743).
Change-Id: I68054644220cd03bc96cf8ffb9c7ee375be77f96
This is to make it behave in a more object orientied way. The
goal is to make it be easier to allow extensions to mark certain
pages as requiring a different policy (For example, CodeEditor
extension uses a blob: url with a WebWorker. We don't want to
include that on the policy of every page, but allow the extension
to mark it as required whenever needed).
This commit does not change code behaviour in any way.
Change-Id: I4bf53dabb6e6c5446cea99a64db68b300cef2fd4
Cleanups include;
- Remove elseif() path for cases where if has a return state
and instead convert the elseif() into an if.
- Fix PHPDoc for non-existent parameter. Corrected parameter
name.
- Add @throws phpdoc annotation for a method that could throw
an exception.
- Remove unnecessary parentheses.
- Make sure line doesn't exceed 100 characters.
Change-Id: Ic2d882ae0c6f3859b5a268b1bfb50c8eafa294d9
Previously domEval didn't have CSP nonces, causing it to violate
the policy.
Also removes the meta tag scheme, as I could not make it compatible
with how RL storage works using domEval instead of real eval() and
it didn't provide much protection anyways.
Bug: T196923
Change-Id: I3cd2d7cc295c39b498d0bf37915d4ba167fdd48c
The current rollout plan calls for initial rollout to only
disallow external JS, and leave removing unsafe inline stuff
to a later date. Thus this adds a useNonces option to the CSP
config to allow that.
Renamed ContentSecurityPolicy::isEnabled() to isNonceRequired
for clarity. The old name has never been in a released version
of MediaWiki, so is removed immediately.
Change-Id: I756d8e97b77c6f97dbbf040a20c8750fecb157c5
This works around a bug in HHVM, where it treats post body as part of
request parameters, even if content-type is application/json.
See https://github.com/facebook/hhvm/issues/6676.
Change-Id: Id54d6657056dee56fc71100dedfb3b53d512eaba
The primary goal here is a defense in depth measure to
stop an attacker who found a bug in the parser allowing
them to insert malicious attributes.
This wouldn't stop someone who could insert a full
script tag (since at current it can't distinguish between
malicious and legit user js). It also would not prevent
DOM-based or reflected XSS for anons, as the nonce value
is guessable for anons when receiving a response cached
by varnish. However, the limited protection of just stopping
stored XSS where the attacker only has control of attributes,
is still a big win in my opinion. (But it wouldn't prevent
someone who has that type of xss from abusing things like
data-ooui attribute).
This will likely break many gadgets. Its expected that any
sort of rollout on Wikimedia will be done very slowly, with
lots of testing and the report-only option to begin with.
This is behind feature flags that are off by default, so
merging this patch should not cause any change in default
behaviour.
This may break some extensions (The most obvious one
is charinsert (See fe648d41005), but will probably need
some testing in report-only mode to see if anything else breaks)
This uses the unsafe-eval option of CSP, in order to
support RL's local storage thingy. For better security,
we may want to remove some of the sillier uses of eval
(e.g. jquery.ui.datepicker.js).
For more info, see spec: https://www.w3.org/TR/CSP2/
Additionally see:
https://www.mediawiki.org/wiki/Requests_for_comment/Content-Security-Policy
Bug: T135963
Change-Id: I80f6f469ba4c0b608385483457df96ccb7429ae5
