This reverts commit 066e77735e.
Reason for revert: Objections brought up on ticket by Esanders are in alignmenet with own objections, that arose after initially providing this patch. We need clarification that is better addressed in new patch, reverting this.
Bug: T270796
Change-Id: Id873b3e3b7efb3003daa4986225b1fb3e7e218c8
Done:
* Replace LanguageConverter::newConverter by LanguageConverterFactory::getLanguageConverter
* Remove LanguageConverter::newConverter from all subclasses
* Add LanguageConverterFactory integration tests which covers all languages by their code.
* Caching of LanguageConverters in factory
* Make all tests running (hope that's would be enough)
* Uncomment the deprecated functions.
* Rename FakeConverter to TrivialLanguageConverter
* Create ILanguageConverter to have shared ancestor
* Make the LanguageConverter class abstract.
* Create table with mapping between lang code and converter instead of using name convention
* ILanguageConverter @internal
* Clean up code
Change-Id: I0e4d77de0f44e18c19956a1ffd69d30e63cf51bf
Bug: T226833, T243332
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
Since MW 1.31 there are messageBox, warningBox, errorBox and successBox
available which should be used instead.
These are simple wrappers around Html::messageBox and related methods
which should be used directly instead. Internal usage has been migrated
as of a9252abb1c, but these public methods remain for now. Once
removed we can also removed the internal Html::infoBox() method.
Bug: T232903
Change-Id: I2756c88987754c4e3f84d98c531d8396098cd7fe
Adding optional $className parameter to provide additional
styling options.
Optional parameter $heading was introduced for MobileFrontend. Therefore
leaving inconsistent parameter order is necessary for the time being.
Bug: T232903
Change-Id: I5857b2c58a47a83156c32f086a73fe2bd48ab0c8
The documentation for the first parameter said it was both "raw HTML"
and "not escaped", but those phrases have opposite meanings. Clarify that
this parameter takes plain text that will be escaped (so not raw HTML),
and rename the parameter from $contents (typically used to mean HTML) to
$text.
Change-Id: I44d6de08a68c493935b2ceff58fac6c51bc45d12
This method is very specific to the installer and is only used in one
place, in WebInstaller, and should probably be moved there.
For now, make its documentation less confusing, more correct, and
mark it as `@internal`, this is not a supported public interface.
Bug: T227297
Change-Id: I8902fe34b80c5152bfd37e9c24427ad48b5f4167
The value of "all" can be completely arbitrary and in practice
it's usually an empty string. Avoid attempting to filter it out
and format anyhow.
Bug: T227202
Change-Id: I371466407bd038914faa4dc3b0cae9547cf427eb
I would argue that these comments do not add any information that
would not be there already. Having them adds mental overhead, because
one needs to read both the comment and the next line of code first to
understand they say the exact same. I don't find this helpful, but
more distracting.
Change-Id: I39c98f25225947ebffdcc2fd8f0243e7a6c070d7
Asking users to select from a list that is in a namespace
other than their user language can be quite difficult.
Transition a number of use cases. This was applied via a flag
to avoid unexpectedly changing extensions also utilizing
the namespace selector.
Pages updated:
* Special:Search
* Special:Watchlist
* Special:RecentChanges
* Special:Contributions
* Special:Import
* Special:WhatLinksHere
Bug: T174057
Change-Id: I3fdac72179a124849ef7ad1e0e54eb66396c3c6e
There are three problems with the CDATA approach:
1. It doesn't work.
HTML5 already interprets the contents of <script> tags as CDATA,
which means escaping of characters like & is not needed. In fact,
in HTML5 mode, a plain script tag with <script>0&1;</script>
would be a syntax error. Indicating it is not interpreted as
text, but as CDATA. Effectively, the only thing an HTML parser
looks for is </script>.
And that's exactly the problem. Producing an inline script
containing the characters "</string>" for legitimate reasons,
is currently broken.
No alternate wrapping or setting can make it work, either.
See also:
https://people.wikimedia.org/~krinkle/200506-html-inlinescript.html
which contains:
<script>/*<![CDATA[*/
if (true && true) {
console.log('This is a <script></script> tag (original)');
}
/*]]>*/</script>
In a browser, the script is terminated by the first "</script>",
leaving the code unfinished, throwing a SyntaxError, and outputting
the rest of the script as plain text on the page.
2. CDATA is only for XML mode, whereas MediaWiki does not support
the XML/XHTML output mode (since MediaWiki 1.22). Instead, we only
output HTML (5). Code that does need to produce XML, should use the
class from Xml.php instead.
3. It gives a false sense of security.
We could just remove the CDATA code as-is and that in itself would be an
improvement per point 2 and 3, and would break nothing per point 1.
However, this commit attempts to address the underlying bug by rejecting
the characters "</script>" from input. If this is needed in a literal,
it is the responsibility of the caller to escape it in a way that is
appropiate for how it is used (string, comment, regex, etc.).
There are two ways this can be used currently in core:
* User input as exported through JSON (e.g. mw.config, or mw.messages).
This is already fine as both FormatJson::encode and json_encode handle
escape either < or / in the string by default already.
* Previews of edits to user scripts. This is currently already broken and
causes the script to end early and produce arbitrary HTML on the page.
This commit limits the impact by refusing to output such script in a
broken way. I will further address that use case in a follow-up.
Bug: T200506
Change-Id: I67ceb34eabf2f62fd3f3841b8f1459289fad28fb
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
Directly use the UTF-8 encoding of the 'NO-BREAK SPACE' (U+00A0) instead of
the HTML/XML entities   or   or .
With the UTF-8 character the generated HTML is shorter and better to read.
Also change the special value for the label in HTMLForm from   to
U+00A0 but also support   for backward compability.
Bug: T154300
Change-Id: I882599ac1120789bb4e524c4394870680caca4f4
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 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
This transformation will find <style> tag with a "data-mw-deduplicate"
attribute. For each value of the attribute, the first instance will be
kept as-is, while any subsequent tags with the same value will be
replaced by a <link rel="mw-deduplicated-inline-style"> with its href
referring to the "data-mw-deduplicate" value using a custom scheme.
This also adds an $attribs parameter to Html::inlineStyle() so the
data-mw-deduplicate attribute can be added.
Note this doesn't actually depend on Ib931e25c, but action=mobileview
will break if it starts being used without that patch.
Bug: T160563
Change-Id: I055abdf4d73ec65771eaa4fe0999ec907c831568
Depends-On: Ib931e25ce85072000e62c486bbe5907f03372494
* Messagebox is now private to Html class to discourage unconventional
usages
* Tests are added for all three helper methods added in
4e7021a231
Bug: T166915
Change-Id: I1c3e4131b2439c0f4fb94ad4e616a909b52d6b78
This will help us consolidate the various uses into one single
method which will help us drive standardisation of these defacto
widgets.
Hopefully, by being a method of the Html class, which has a very
low barrier for use will drive down the inconsistent display of
warning/error boxes across MediaWiki's products
Various usages of warningbox and errorbox have been ported over.
I've retained some more complicated usages which make use of the
parser (wrapWikiMsg) and any where id and class are medled with
- we'll probably want to consider whether we want to encourage
those going forward as they encourage adjusting the styling.
Bug: T166915
Change-Id: I2757e1f4ff2599e93a7257fc644cab69063896d2
Replaces a long excuse in Html class not to call
Sanitizer::encodeAttribute() with a call to it.
In anything security related, excuses are a sign that you're doing
something wrong:)
Change-Id: Icf7f60d8cd6ea757d8f1999d638b82733001f68a
It's unreasonable to expect newbies to know that "bug 12345" means "Task T14345"
except where it doesn't, so let's just standardise on the real numbers.
Change-Id: I6f59febaf8fc96e80f8cfc11f4356283f461142a
We blacklisted them in 2010. Modern browsers support them fairly well,
and it doesn't seem to conflict with any of our code.
I tested this with SecurePoll poll creation form, which contains an
astonishingly wide range of form controls and validation options.
Change-Id: I08244addcf9b6eb96137895f28e7b750914fef5c
Use HTTPS instead of HTTP where the HTTP link is a redirect to the HTTPS link.
Also update some defect links.
Change-Id: Ic3a5eac910d098ed5c2a21e9f47c9b6ee06b2643
`<command>` is nowhere to be found in current HTML5 specification.
Scarce documentation on the internet hints that it has been removed.
Change-Id: I2a704194c7e8f8ca307f9d97c7f47a47cfaf00a6
This is to prevent people from closing the <style> tag, and
then doing arbitrary js-y things. In particular, this is needed
for when previewing user css pages.
This does not escape '>' since its used as the child selector
in css, and generally speaking, '>' is safe inside the contents
of elements.
Bug: T133147
Change-Id: If024398d7bd4b578ad7f8c74367787f5b19eb9d7
Follows-up a464d1d41 which changed OutputPage::headElement()
to join pieces by a line break instead of hardcoding line breaks
after (some) generated pieces.
This caused a minor regression in the form of a blank line between
<html> and <head> on every page, because I missed the one that
came from this class.
Change-Id: I5e48b852809699b205f4581c833605f3e232610a
tl;dr: Having unnessary complexity in security critical code is bad.
* Extra options add extra complexity and maintenance burden
** Thus we should only have one html output mode. well formed = false
was already vetoed in T52040, so lets go with WellFormed=true.
* Options which are used by very few people tend to get tested less
* Escaping is an area of code where we should be very conservative
* Having escaping rules depend on making assumptions about which
characters various browsers consider "whitespace" is scary
* $wgWellFormedXml=false has had a negative security impact in the
past (Usually not directly its fault, but has made other bugs
more exploitable)
* Saving a couple bytes (even less bytes after gzip taken into
account) is really not worth it in this context (imho).
Change-Id: I5c922e0980d3f9eb39adb5bb5833e158afda42ed
* Remove trailing space in self-closing tag.
Brings parserTest output of Parser and Parsoid closer together.
* Remove various line breaks at begin and end of script contents.
* Remove FILTER_NOMIN from makeConfigSetScript() output.
This isn't part of any user- or page-dependent module and not minified.
And Xml::encodeJsCall already ensures compact output for prod mode.
Bug: T127328
Change-Id: I85a5a59fd0955c1a112e8b24b933f0d9e983a156
Already stripped by Html::element before actual output, but
remove them from the attribute arrays as well.
Change-Id: I8699ca7bf40df07e9d4c370f6863003c095ced0e
Remove empty line comments as found by the
MediaWiki.WhiteSpace.SpaceBeforeSingleLineComment.EmptyComment sniff
Change-Id: I5d694f7a7d3bc97e16300ba03c60ad17f3c912a5
