Reduce our reliance on static state and configuration, and
propagate more state in explicit ways, through context, and
request parameters.
OutputPage creates ResourceLoaderContext and ResourceLoaderClientHtml
based on the configuration (via ResourceLoader::inDebugMode).
Everything within those classes should not need to check it
again.
* ResourceLoaderClientHtml:
Already doesn't check MW config, but it's test was still
mocking it. Removed now, and confirmed that it passes both
with true and false. The individual test cases set
debug=true/false as needed already.
It's sets were previously relying on the accidental behaviour
that within a unit test, we don't serialise over HTTP, which
meant that a pure PHP boolean would survive. With the new
raw `=== 'true'` check, this no longer works. Set it as a
string explicitly instead, which is the only thing we support
outside unit tests as well.
* ResourceLoaderContext:
Remove fallback to MW config when 'debug' is unset.
This is never unset in practice given that all load.php
urls have it set by OutputPage based on ResourceLoader::inDebugMode.
This change means that manually constructed ad-hoc load.php
urls that are missing 'debug=' parameter, will now always be
read as debug=false. This was the default already, but could
previously be changed through wgResourceLoaderDebug.
When changing wgResourceLoaderDebug, everything will still have
debug=true as before. The only change is when constructing load.php
urls manually and explicitly not set it.
Bug: T32956
Change-Id: Ie3424be46e2b8311968f3068ca08ba6a1139224a
It also removes some code duplication which is nice.
This unlocks various future changes, including:
* Making the `$config` parameter mandatory for the ResourceLoader class
constructor, which currently falls back to global state.
This should be deprecated and removed.
* Making it possible to instantiate the ResourceLoader class
without all the default MW modules being registered from
global state. E.g. move MW module registration from main class
constructor to ServiceWiring, and remove the 'EmptyResourceLoader'
class hack from unit tests, and use regular 'new ResourceLoader'
instead.
* Making ResourceLoader a standalone library (some day),
e.g. allowing it to be instantiated from a basic PHP script,
in a way that is still useful and perhaps able to serve
(most) RL modules without MW itself.
Bug: T32956
Change-Id: I4939f296c705b268e9cf8de635e923a739410470
Package files are files that are part of a module, but are not
immediately executed when the module executes. Instead, they are
lazy-excecuted when require() is called on them. Package files can be
scripts (JS) or data (JSON), and can be real files on the file system,
or virtual files generated by a callback.
Using virtual data files, server-side data and config variables can be
bundled with a module. Support for file-based require() allows us to
import npm modules into ResourceLoader more easily.
The require function passed to each script execution context, which was
previously a reference to the global mw.loader.require() function, is
changed to one that is scoped to the module and the file being executed.
This is needed to support relative paths: require( '../foo.js' ) can
mean a different file depending on the path of the calling file.
The results of require()ing each file (i.e. the value of module.exports
after executing it) are stored, and calling require() on the same file a
second time won't execute it again, but will return the stored value.
Miscellaneous changes:
- Add XmlJsCode::encodeObject(), which combines an associative array of
XmlJsCode objects into one larger XmlJsCode object. This is needed for
encoding the packageFiles parameter in mw.loader.implement() calls.
Bug: T133462
Change-Id: I78cc86e626de0720397718cd2bed8ed279579112
This was originally a global search and replace. I manually checked all
replacements and reverted them if (due to the lack of type hints) either
null (that would be 0 when counted) or a Countable object can end in the
variable or property in question.
Now this patch only touches places where I'm sure nothing can break.
For the sanity of the honorable reviewers this patch is exclusively touching
negated counts. You should not find a single `!== []` in this patch, that
would be a mistake.
Change-Id: I5eafd4d8fccdb53a668be8e6f25a566f9c3a0a95
Should be "string" not "String" and "array" not "Array" in
@param, @return and @var use cases. Also, minor typo fixes.
Change-Id: I9d5ebc5b741c6560907b95f7c0c4039da2861f4a
If something puts a string that's invalid UTF-8 in a JS config variable,
JSON serialization will fail on the entire config blob. Currently, this
causes the entire config blob to be silently dropped, which breaks all
JavaScript because elementary variables like wgPageName are missing.
This change makes this scenario fail loudly rather than quietly, by
throwing an exception. This also makes bugs like these easier to track
down.
Bug: T206475
Change-Id: Ief2ae00228389a23627d440dc1cd9a54cf2b6926
We already had a $filterCacheVersion variable, but it was
only used for the internal cache for JS and CSS minification,
which is not enough. If there is a breaking change in either
of these processes, we also need to invalidate version hashes.
This commit renames ResourceLoader::$filterCacheVersion to
ResourceLoader::CACHE_VERSION and takes it into account in
getVersionHash(). Adding it to getDefinitionSummary() is not
sufficient, because content-hashed modules also need to be
invalidated when there's a breaking change in the minifiers.
This cache version can also be incremented when there's a
breaking change in image embedding or LESS compilation,
although content hashing deals with that already, so we
could also add a separate cache version for those that's
only added to getDefinitionSummary().
Bug: T176884
Change-Id: Ife6efa71f310c90b9951afa02212b2cb6766e76d
* There is only a single non-test caller to this method in the entire
codebase, and no callers elsewhere (Wikimedia Git, Codesearch).
It's only used with an array, so remove the other unused code paths,
and mark it internal (to my knowledge, nothing ever used it outside
RL in the past, either).
* Add test coverage for the module indexing logic.
Change-Id: I9e0f95416d5b2fdd87323288231ee6d8c85d88e7
Follows-up dc3fc6cf81, which documented the reasoning for the
specific json flags in StartupModule. In wanting to re-use them
in a different module it became apparant that perhaps it was a
bit too conservative in only allowing the flags to be used in
a script HTTP response.
Lax the contract a little bit (that is, do more escaping) to also
allow safe re-use in HTML context.
I considered making these separate methods (e.g. forScriptResponse,
and forInlineScript) but decided not to because the vast majority
of callers would have to be forInlineScript eventhough the code
in question had no responsiblity or knowledge of it becoming an
inline script, because ResourceLoader allows most modules to
become embedded if they support a private or preview mode.
Those modes are implemented by calling makeModuleResponse, and
wrapping it an inline script. It seems appealing and simplifying
to the API to require that script output is always safe for
embedding rather than complicating the API for winning back
a literal handful of bytes in the edge case that a user-generated
string contains a '<' and was not embedded. I estimate that with
gzip, it will literally save only a single byte, even if used
multiple times. Let's focus optimisation efforts elsewhere :)
Change-Id: I7742dabba6750deecf6fbf51cf9a77ee8cbfc727
Added spaces around .
Removed empty return statement which are not required
Removed return after phpunit markTestIncomplete,
which is throwing to exit the test, no need for a return
Change-Id: I2c80b965ee52ba09949e70ea9e7adfc58a1d89ce
Follows-up a0809dfa5 (2015) which removed the feature for which
this would be used. No callers in MediaWiki core, Wikimedia Git,
or elsewhere indexed by Codesearch.
Bug: T65240
Change-Id: I56af8df3d072cd76a38595254812af6be268b134
The PHP interface still supports it without deprecation, but there
is no need for the client to support it given it is only ever called
from the startup module, which always passes it an object.
Follows-up 6815e35575, which did the same for `state(key, val)`.
Also simplify the implementation to avoid extra calls and optimise
for the only signature.
Mark the method as @private, and change the existing use of
@protected to @private (doesn't really make sense in JavaScript).
Change-Id: I24786f90bcfb256b2e7c37f7760bba1a5e399443
This significantly simplifies the getVersionHash implementation for
StartupModule, and fixes a couple of bugs.
Previously, the startup module's E-Tag was determined by the
'getDefinitionSummary' method, which combined the E-Tag values
from all registered modules, plus what we thought is all information
used by 'getScript' (config vars, embedded script files, list
of base modules, ...)
However, this were various things part of the manifest that it
forgot about, including:
* Changes to the list of dependencies of a module.
* Changes to the name of module.
* Changes to the cache group of module.
* Adding or removing a foreign module source (mw.loader.addSource).
These are all quite rare, and when they do change, they usually
also involve a change that *was* tracked already. But, sometimes
they don't and that's when bugs happened.
Instead of the tracking array of getDefinitionSummary, we now
use the 'enableModuleContentVersion' option for StartupModule,
which simply calls the actual getScript() method and hashes that.
Of note: When an exception happens with the version computation of
any individual module, we catch it, log it, and continue with the
rest. Previously, the first time such error was discovered at
run-time would be in the getCombinedVersion() call from
StartupModule::getAllModuleHashes(). That public getCombinedVersion()
method of ResourceLoader had the benefit of also outputting details
of that exception in the HTTP response output. In order to keep that
behaviour, I made outputErrorAndLog() public so that StartupModule
can call it directly now. This is covered by
ResourceLoaderTest::testMakeModuleResponseStartupError.
Bug: T201686
Change-Id: I8e8d3a2cd2ccd68d2d78e988bcdd0d77fbcbf1d4
* Remove use of MediaWiki's FormatJson class. Ref T32956.
* FormatJson::decode() was a direct alias of json_decode.
* FormatJson::encode() is a wrapper for json_encode that
sets `JSON_UNESCAPED_SLASHES | JSON_HEX_TAG | JSON_HEX_AMP`
by default, plus a boolean parameter to add JSON_PRETTY_PRINT.
Instead, use json_encode() directly. By default json_encode() escapes
slashes and non-ASCII unicode.
* Audit our uses of JSON encoding, document their needs, and set flags
as needed.
* Remove $mapToJson abstraction from ResourceLoaderStartUpModule.
* Always pretty-print the list of module names in the error message,
regardless of debug mode.
* Remove hacky indentation-adder from ResourceLoaderStartUpModule.
This was relying on there not being line breaks in the json output
outside debug mode, and adding a level of indentation to roughly
match the code in the JSON is substituted. This didn't match and
just generally doesn't seem worth it.
Change-Id: I3e09ddeb4d53c8980195c1855303c0ee25bd4c20
The use of global variables was deprecated in favour of
ResourceLoaderModule::getLessVars() on a per-module basis.
Also moved testLessFileCompilation case to the appropiate file as it
covers ResourceLoaderFileModule.php, not ResourceLoader.php.
Bug: T140804
Depends-On: Ib1b2808df2384473bfac47f53a5d25d7c9bbca2b
Depends-On: I96047f69d01c4736306df2719267e6347daf556f
Change-Id: If708087c85c80355c7e78f1768529b5f2e16ed07
This has no usage in non-test code anywhere in Wikimedia Git,
and was only used in the test suite for mw.loader in core,
and in the test suite for the NavigationTiming extension.
The core usage is as part of this commit. The usage in NavTiming
is updated by I240ced4e65988f9.
Bug: T127328
Depends-On: I240ced4e65988f96c7ece3772378c2c9a335fb9a
Change-Id: Ic17c797e528feaf07a4777709d705615fea353e5
This commit implements step 4 and step 5 of the plan outlined at T192623.
Before this task began, the typical JavaScript execution flow was:
* HTML triggers request for startup module (js req 1).
* Startup module contains registry, site config, and triggers
a request for the base modules (js req 2).
* After the base modules arrive (which define jQuery and mw.loader),
the startup module invokes a callback that processes RLQ,
which is what will request modules for this page (js req 3).
In past weeks, we have:
* Made mediawiki.js independent of jQuery.
* Spun off 'mediawiki.base' from mediawiki.js – for everything
that wasn't needed for defining `mw.loader`.
* Moved mediawiki.js from the base module request to being embedded
as part of startup.js.
The concept of dependencies is native to ResourceLoader, and thanks to the
use of closures in mw.loader.implement() responses, we can download any
number of interdependant modules in a single request (or parallel requests).
Then, when a response arrives, mw.loader takes care to pause or resume
execution as-needed. It is normal for ResourceLoader to batch several modules
together, including their dependencies.
As such, we can eliminate one of the two roundtrips required before a
page can request modules. Specifically, we can eliminate "js req 2" (above),
by making the two remaining base modules ("jquery" and "mediawiki.base") an
implied dependency for all other modules, which ResourceLoader will naturally
fetch and execute in the right order as part of the batch request.
Bug: T192623
Change-Id: I17cd13dffebd6ae476044d8d038dc3974a1fa176
Switch from passing a plain function to RLQ that calls depends
on using() from 'mediawiki.base' and 'jquery', to using the new
array format introduced in Ica7bb9c3bdb (T192623).
This ensures the code will keep after we change the fact that
RLQ is blocked by base modules.
Bug: T192623
Change-Id: Id50e6dcc28c13b021d8395a2da0d83b46f88e18c
Find: /isset\(\s*([^()]+?)\s*\)\s*\?\s*\1\s*:\s*/
Replace with: '\1 ?? '
(Everywhere except includes/PHPVersionCheck.php)
(Then, manually fix some line length and indentation issues)
Then manually reviewed the replacements for cases where confusing
operator precedence would result in incorrect results
(fixing those in I478db046a1cc162c6767003ce45c9b56270f3372).
Change-Id: I33b421c8cb11cdd4ce896488c9ff5313f03a38cf
The last remaining users of this feature were MobileFrontend and Minerva,
which have been migrated to direct imports.
Bug: T140807
Change-Id: I1a66a2ad314bde332297798520e5ec3e0e3d4c9b
This prevents cache churn when the wiki-global LESS variables vary
between wikis because the cache key is used as a "global" instead of
db-local. This is good for the common case, but should still explicitly
vary if the vars differ between wikis.
Bug: T191937
Change-Id: If12fd07a7062792205384150d6f5fd9a83f996cc
* Fix ResourceLoaderClientHtml to return what it was documented to
return, a WrappedStringList. It accidentally used the wrong join()
method, causing it to create a plain string too early.
* Update method documentations from ClientHtml::getBodyHtml to
BaseTemplate::getTrail for 'bottomscripts', and from
ResourceLoader::makeInlineScript for 'reporttime'.
* Update BaseTemplate::getTrail to join by new line instead of native string
concatenation.
This by itself would suffice for the most common case, but in order
to also account for the possibility of extensions using hooks for
'SkinAfterBottomScripts' that concatenate, update bottomScripts()
to pass a plain string to the hook and merge it later.
Change-Id: If0d227cb9db67229a27d489c082db790ea8e3840
Follows-up 70941efd35 which broke various public
signatures of the ClientHtml class that I'd prefer to handle
differently.
This commit mainly restores support for all previously public
signatures, and either removes the need for a parameter, or moves
it to the end of the original signature (as optional param).
* ClientHtml::getHeadHtml: Remove the positional/required parameter
that was added. Restoring the method to being a stateless computer
that requires no parameters. Pass the option via construct instead.
* ClientHtml::makeLoad:
- Make $nonce optional.
- Restore $extraQuery as optional.
* ResourceLoader::makeInlineScript: Document $nonce as optional
(matching the implementation).
Change-Id: Iaf33f2a060048e6606fba8d875b6d2953b21ef45
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
Deprecated since 1.30, only had one caller (MobileFrontend)
which has been fixed since.
Bug: T140804
Change-Id: I39b594e3082ebe321bae463780cfadbfb3de1d5c
* Move buildModulesString() call from doRequest() to batchRequest()
This keeps all module string "packing" logic located to the same function,
which is batchRequest(). It also means that the moduleMap object will not
leave the function, which helps in maintenance given it's very internal.
* Add comments to all the methods referring to each other.
* Explain why buildModulesString() is only a partial port, and the rest is
inlined in batchRequest().
* Minor changes to the JS and PHP implementation to better match each other.
- '$groups' -> '$moduleMap'.
- Remove redundant '$str'.
Bug: T188076
Change-Id: I7b0790606c456e492ab682faeb80f7e7fce7d9f8
It was deprecated to encourage callers to use makeInlineScript()
instead, because before makeInlineScript() was introduced, callers
would often manually wrap it in a <script> tag, which we don't want
because 1) We want de-duplication through WrappedString and 2)
callers sometimes forget to use Html.php methods that ensure
proper escaping as-needed.
However, there is still use in getting thet wrapped script without
also the wrapped HTML script tag. Namely, for use in http responses
that produce JavaScript, rather than HTML, where there cannot be
an HTML script tag.
There is at least one such caller (in SpecialJavaScriptTest) and
it's fine for there to be others.
Change-Id: Icfc285e12293c76dc68a220deb6816e32b058209
Follows-up b7eb243d92, which changed minification to be per-module,
but had as result that the little implement() wrapper is not always
minified, e.g. when outputting modules that have minification disabled
(such as 'user.tokens' and 'user.options').
Rather than introducing some complex minification cache for the
wrapper (with a find and replace for the variable content), simply
add a hardcoded minified version of the wrapper.
Change-Id: Iccf0d3408beab4387031cc55689394ff67e1e64b
