There is a common and reasonable need for longer lines in tests.
The nudge for shorter lines doesn't seem valuable here. The natural
breaks will likely still fall in 80-100 given the enforced practice
for non-test code, e.g. whether through habit, or 80-100 column markers
in text editors, or the finite width of diff and code review
interfaces.
Change-Id: I879479e13551789a67624ce66f0946d2f185e6ee
* parent::setUp() should be first, and ::tearDown()
should be last
* Move tests that directly extend PHPUnit\Framework\TestCase
to /unit
Change-Id: I1172855c58f4f52a8f624e6d596ec43beb8c93ff
The name change happened some time ago, and I think its
about time to start using the name name!
(Done with a find and replace)
My personal motivation for doing this is that I have started
trying out vscode as an IDE for mediawiki development, and
right now it doesn't appear to handle php aliases very well
or at all.
Change-Id: I412235d91ae26e4c1c6a62e0dbb7e7cf3c5ed4a6
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
Note that some (not all) of these are flipped, listing the expected
value second. I guess this is because it's like this in QUnit.
This patch also replaces loose assertEquals() checks with the more
strict assertSame(), which should be preferred when comparing strings.
Change-Id: I5bb27a6b63ab1642fab60483647d6b663100b144
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
With "$wgResourceBasePath = '/';", various ContentSecurityPolicy tests
failed due to unexpected output. An extra "extensions" is added in the
output line.
The reason is getAdditionalSelfUrls() injecting URls from a few global
settings but the test fails to set them in setUp(). The settings are:
$wgLoadScript
$wgExtensionAssetsPath
$wgStylePath
$wgResourceBasePath
Set them explicitly in setUp() so the test outcome does not depend on
values that might have been set in LocalSettings.php.
Add a quick test to ensure getAdditionalSelfUrls() does recognize
domains in those four global settings.
Change-Id: Ia0dc2f44c71bdf89a0ee9ef82d9cb6a1cbd8a9da
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
