jenkins-bot has submitted this change and it was merged.
Change subject: Add property filtering
......................................................................
Add property filtering
Properties listed in $wgTemplateStylesPropertyBlacklist, or
those that contain function-like values not listed in
$wgTemplateStylesFunctionWhitelist cause the containing
declaration to be omitted from rendering entirely.
Additionally, rule selectors are unconditionally prepended
with '#mw-content-text' so that they cannot be applied to
UI elements outside the actual page content.
Change-Id: Id3d7dff465363d0163e4a5a1f31e770b4b0a67e2
---
M CSSParser.php
M CSSRenderer.php
M TemplateStyles.hooks.php
M extension.json
R tests/phpunit/CSSParseRenderTest.php
5 files changed, 124 insertions(+), 47 deletions(-)
Approvals:
BryanDavis: Looks good to me, approved
Brion VIBBER: Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/CSSParser.php b/CSSParser.php
index 67abfc2..889d0cd 100644
--- a/CSSParser.php
+++ b/CSSParser.php
@@ -30,24 +30,24 @@
(?# Sequences of whitespace )
| \/\* (?: [^*]+ | \*[^\/] )* \*\/ [ \n\t]*
(?# Comments and any trailing whitespace )
- | " (?: [^"\\\\\n]+ | \\\\\. )* ["\n]
+ | " (?: [^"\\\\\n]+ | \\\\. )* ["\n]
(?# Double-quoted string literals (to newline
when unclosed )
- | \' (?: [^\'\\\\\n]+ | \\\\\. )* [\'\n]
- (#? Single-quoted string literals (to newline
when unclosed )
+ | \' (?: [^\'\\\\\n]+ | \\\\. )* [\'\n]
+ (?# Single-quoted string literals (to newline
when unclosed )
| [+-]? (?: [0-9]* \. )? [0-9]+ (?: [_a-z][_a-z0-9-]* |
% )?
- (#? Numerical literals - including optional
trailing units or percent sign )
+ (?# Numerical literals - including optional
trailing units or percent sign )
| @? -? (?: [_a-z] | \\\\[0-9a-f]{1,6} [ \n\t]? )
(?: [_a-z0-9-]+ | \\\\[0-9a-f]{1,6} [ \n\t]? |
[^\0-\177] )* (?: [ \n\t]* \( )?
- (#? Identifiers - including leading `@` for
at-rule blocks )
- (#? Trailing open captures are captured to
match functional values )
+ (?# Identifiers - including leading `@` for
at-rule blocks )
+ (?# Trailing open captures are captured to
match functional values )
| \# (?: [_a-z0-9-]+ | \\\\[0-9a-f]{1,6} [ \n\t]? |
[^\0-\177] )*
- (#? So-called hatch literals )
+ (?# So-called hatch literals )
| u\+ [0-9a-f]{1,6} (?: - [0-9a-f]{1,6} )?
- (#? Unicode range literals )
+ (?# Unicode range literals )
| u\+ [0-9a-f?]{1,6}
- (#? Unicode mask literals )
+ (?# Unicode mask literals )
| .)
- (#? Any unmatched token is reduced to single
characters )
+ (?# Any unmatched token is reduced to single
characters )
/xis', $css, $match );
$space = false;
@@ -69,8 +69,13 @@
// prevents trying to obfuscate ASCII in
identifiers to prevent matches.
$t = preg_replace_callback(
'/\\\\([0-9a-f]{1,6})[ \n\t]?/', function( $match ) {
- return html_entity_decode(
'&#'.$match[1].';', ENT_NOQUOTES, 'UTF-8' );
+ return html_entity_decode(
'&#x'.$match[1].';', ENT_NOQUOTES, 'UTF-8' );
}, $t );
+
+ // close unclosed string literals
+ if ( preg_match( '/^([\'"])(.*)\n$/', $t,
$match ) ) {
+ $t = $match[1] . $match[2] . $match[1];
+ }
$space = false;
$this->tokens[] = $t;
@@ -95,7 +100,7 @@
$this->index += $num;
return $text;
}
- return '';
+ return [];
}
private function consumeTo( $delim ) {
@@ -129,7 +134,7 @@
$this->consumeWS();
if ( $this->peek( 0 )!=':' ) {
$this->consumeTo( [';', '}', null] );
- if ( $this->peek( 0 ) == ';' ) {
+ if ( $this->peek( 0 ) ) {
$this->consume();
$this->consumeWS();
}
@@ -164,9 +169,6 @@
}
}
}
- if ( $this->peek( 0 ) == '}' ) {
- $this->consume();
- }
return $decls;
}
@@ -182,13 +184,15 @@
* Returns:
* [ selectors => [ selector* ], decls => [ decl*
] ]
*/
- public function parseRule() {
+ public function parseRule( $baseSelectors ) {
$selectors = [];
$text = '';
$this->consumeWS();
while ( !in_array( $this->peek( 0 ), ['{', ';', null] ) ) {
if ( $this->peek( 0 ) == ',' ) {
- $selectors[] = $text;
+ if ( $text != '' ) {
+ $selectors[] = $baseSelectors . $text;
+ }
$this->consume();
$this->consumeWS();
$text = '';
@@ -196,10 +200,15 @@
$text .= $this->consume()[0];
}
}
- $selectors[] = $text;
+ if ( $text != '' ) {
+ $selectors[] = $baseSelectors . $text;
+ }
if ( $this->peek( 0 ) == '{' ) {
$this->consume();
return [ "selectors"=>$selectors,
"decls"=>$this->parseDecls() ];
+ }
+ if ( $this->peek( 0 ) ) {
+ $this->consume();
}
return null;
}
@@ -225,13 +234,13 @@
* Returns:
* [ [ name=>ATIDENT? , text=>body? ,
rules=>rules? ]* ]
*/
- public function rules( $end = [ null ] ) {
+ public function rules( $baseSelectors = '', $end = [ null ] ) {
$atrules = [];
$rules = [];
$this->consumeWS();
while ( !in_array( $this->peek( 0 ), $end ) ) {
- if ( in_array( $this->peek( 0 ), [ '@media' ] ) ) {
- $at = $this->consume();
+ if ( in_array( strtolower( $this->peek( 0 ) ), [
'@media' ] ) ) {
+ $at = $this->consume()[0];
$this->consumeWS();
$text = '';
while ( !in_array( $this->peek( 0 ), ['{', ';',
null] ) ) {
@@ -239,7 +248,7 @@
}
if ( $this->peek( 0 ) == '{' ) {
$this->consume();
- $r = $this->rules( [ '}', null ] );
+ $r = $this->rules( $baseSelectors, [
'}', null ] );
if ( $r ) {
$atrules[] = [ "name"=>$at,
"text"=>$text, "rules"=>$r ];
}
@@ -247,7 +256,7 @@
$atrules[] = [ "name"=>$at,
"text"=>$text ];
}
} elseif ( $this->peek( 0 )[0] == '@' ) {
- $at = $this->consume();
+ $at = $this->consume()[0];
$text = '';
while ( !in_array( $this->peek( 0 ), ['{', ';',
null] ) ) {
$text .= $this->consume()[0];
@@ -261,14 +270,19 @@
} else {
$atrules[] = [ "name"=>$at,
"text"=>$text ];
}
+ } elseif ( $this->peek( 0 ) == '}' ) {
+
+ $this->consume();
+
} else {
- $rules[] = $this->parseRule();
+ $rules[] = $this->parseRule( $baseSelectors );
}
$this->consumeWS();
}
if ( $rules ) {
$atrules[] = [ "name"=>'', "rules"=>$rules ];
}
+ $this->consumeWS();
if ( $this->peek( 0 ) !== null ) {
$this->consume();
}
diff --git a/CSSRenderer.php b/CSSRenderer.php
index 5135666..70d2631 100644
--- a/CSSRenderer.php
+++ b/CSSRenderer.php
@@ -32,7 +32,7 @@
}
foreach ( $rules as $at ) {
- switch ( $at['name'] ) {
+ switch ( strtolower( $at['name'] ) ) {
case '@media':
if ( $media == '' ) {
$this->add( $at['rules'],
"@media ".$at['text'] );
@@ -51,7 +51,7 @@
*
* @return string Rendered CSS
*/
- function render() {
+ function render( $functionWhitelist = [], $propertyBlacklist = [] ) {
$css = '';
@@ -60,11 +60,29 @@
$css .= "$at {\n";
}
foreach ( $rules as $rule ) {
- $css .= implode( ',', $rule['selectors'] ) .
"{";
- foreach ( $rule['decls'] as $key => $value ) {
- $css .= "$key:" . implode( '', $value )
. ';';
+ if ( $rule
+ and array_key_exists( 'selectors',
$rule )
+ and array_key_exists( 'decls', $rule ) )
+ {
+ $css .= implode( ',',
$rule['selectors'] ) . "{";
+ foreach ( $rule['decls'] as $key =>
$value ) {
+ if ( !in_array( strtolower(
$key ), $propertyBlacklist ) ) {
+ $blacklisted = false;
+ foreach ( $value as
$prop ) {
+ if (
preg_match( '/^ ([^ \n\t]+) [ \n\t]* \( $/x', $prop, $match ) ) {
+ if (
!in_array( strtolower( $match[1] ), $functionWhitelist ) ) {
+
$blacklisted = true;
+
break;
+ }
+ }
+ }
+ if ( !$blacklisted ) {
+ $css .= "$key:"
. implode( '', $value ) . ';';
+ }
+ }
+ }
+ $css .= "} ";
}
- $css .= "} ";
}
if ( $at != '' ) {
$css .= "} ";
diff --git a/TemplateStyles.hooks.php b/TemplateStyles.hooks.php
index b75e356..375ccc8 100644
--- a/TemplateStyles.hooks.php
+++ b/TemplateStyles.hooks.php
@@ -15,9 +15,18 @@
return true;
}
- public static function onUnitTestsList( &$paths ) {
- $paths[] = __DIR__ . '/tests/phpunit/';
+ /**
+ * Add phpunit tests
+ *
+ * @param array &$files List of phpunit test files
+ */
+ public static function onUnitTestsList( &$files ) {
+ $files[] = __DIR__ . '/tests/phpunit/';
return true;
+ }
+
+ public static function makeConfig() {
+ return new GlobalVarConfig( 'TemplateStyles' );
}
private static function decodeFromBlob( $blob ) {
@@ -33,17 +42,12 @@
}
public static function onOutputPageParserOutput( &$out, $parseroutput )
{
- global $wgTemplateStylesNamespaces;
- if ( $wgTemplateStylesNamespaces ) {
- $namespaces = $wgTemplateStylesNamespaces;
- } else {
- $namespaces = [ NS_TEMPLATE ];
- }
+ $config = ConfigFactory::getDefaultInstance()->makeConfig(
'TemplateStyles' );
$renderer = new CSSRenderer();
$pages = [];
- foreach ( $namespaces as $ns ) {
+ foreach ( self::getConfigArray( $config, 'Namespaces' ) as $ns
) {
if ( array_key_exists( $ns,
$parseroutput->getTemplates() ) ) {
foreach ( $parseroutput->getTemplates()[$ns] as
$title => $pageid ) {
$pages[$pageid] = $title;
@@ -69,18 +73,39 @@
}
- $selfcss = $out->getProperty( 'templatestyles' );
+ $selfcss = $parseroutput->getProperty( 'templatestyles' );
if ( $selfcss ) {
- $selfcss = self::decodeFromBlob( unserialize( gzdecode(
$selfcss ) ) );
+ $selfcss = self::decodeFromBlob( $selfcss );
if ( $selfcss ) {
$renderer->add( $selfcss );
}
}
- $css = $renderer->render();
+ $css = $renderer->render(
+ self::getConfigArray( $config, 'FunctionWhitelist' ),
+ self::getConfigArray( $config, 'PropertyBlacklist' )
+ );
if ( $css ) {
$out->addInlineStyle( $css );
}
+ }
+
+ /**
+ * Convert a object-style configuration value to a plain array by
+ * returning the array keys from the found configuration where the
+ * associated value is truthy.
+ *
+ * @param Config $config Configuration instance
+ * @param string $name Name of configuration option
+ * @return array Configuration data
+ */
+ private static function getConfigArray( Config $config, $name ) {
+ return array_keys( array_filter(
+ $config->get( "TemplateStyles{$name}" ),
+ function ( $val ) {
+ return (bool)$val;
+ }
+ ) );
}
/**
@@ -101,7 +126,10 @@
$css = new CSSParser( $input );
if ( $css ) {
- $parser->getOutput()->setProperty( 'templatestyles',
self::encodeToBlob( $css->rules() ) );
+ $parser->getOutput()->setProperty(
+ 'templatestyles',
+ self::encodeToBlob( $css->rules(
'#mw-content-text ' ) )
+ );
}
// TODO: The UX would benefit from the CSS being run through the
@@ -112,7 +140,7 @@
. Html::rawElement(
'p',
[ 'class' => 'mw-templatestyles-caption' ],
- wfMessage( 'templatedata-doc-title' ) )
+ wfMessage( 'templatestyles-doc-title' ) )
. Html::element(
'pre',
[ 'class' => 'mw-templatestyles-stylesheet' ],
diff --git a/extension.json b/extension.json
index 13f787a..03ab87a 100644
--- a/extension.json
+++ b/extension.json
@@ -37,9 +37,26 @@
"OutputPageParserOutput": [
"TemplateStylesHooks::onOutputPageParserOutput"
],
- "UnitTestList": [
- "TemplateStyleHooks::onUnitTestList"
+ "UnitTestsList": [
+ "TemplateStylesHooks::onUnitTestsList"
]
+ },
+ "config": {
+ "TemplateStylesNamespaces": {
+ "10": true
+ },
+ "TemplateStylesFunctionWhitelist": {
+ "rgb": true
+ },
+ "TemplateStylesPropertyBlacklist": {
+ "url": true,
+ "behavior": true,
+ "-moz-binding": true,
+ "-o-link": true
+ }
+ },
+ "ConfigRegistry": {
+ "TemplateStyles": "TemplateStylesHooks::makeConfig"
}
}
diff --git a/tests/CSSParseRenderTest.php b/tests/phpunit/CSSParseRenderTest.php
similarity index 100%
rename from tests/CSSParseRenderTest.php
rename to tests/phpunit/CSSParseRenderTest.php
--
To view, visit https://gerrit.wikimedia.org/r/282424
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Id3d7dff465363d0163e4a5a1f31e770b4b0a67e2
Gerrit-PatchSet: 18
Gerrit-Project: mediawiki/extensions/TemplateStyles
Gerrit-Branch: master
Gerrit-Owner: coren <m...@uberbox.org>
Gerrit-Reviewer: Brion VIBBER <br...@wikimedia.org>
Gerrit-Reviewer: BryanDavis <bda...@wikimedia.org>
Gerrit-Reviewer: Jdlrobson <jrob...@wikimedia.org>
Gerrit-Reviewer: Jforrester <jforres...@wikimedia.org>
Gerrit-Reviewer: Legoktm <legoktm.wikipe...@gmail.com>
Gerrit-Reviewer: Tim Starling <tstarl...@wikimedia.org>
Gerrit-Reviewer: coren <m...@uberbox.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
