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

CSSParser.php

@@ -30,24 +30,24 @@ class CSSParser {
 				(?# 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 @@ class CSSParser {
 				// 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 @@ class CSSParser {
 			$this->index += $num;
 			return $text;
 		}
-		return '';
+		return [];
 	}
 
 	private function consumeTo( $delim ) {
@@ -129,7 +134,7 @@ class CSSParser {
 		$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 @@ class CSSParser {
 				}
 			}
 		}
-		if ( $this->peek( 0 ) == '}' ) {
-			$this->consume();
-		}
 		return $decls;
 	}
 
@@ -182,13 +184,15 @@ class CSSParser {
 	 * 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,11 +200,16 @@ class CSSParser {
 				$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 @@ class CSSParser {
 	 * 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 @@ class CSSParser {
 				}
 				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 @@ class CSSParser {
 					$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 @@ class CSSParser {
 				} 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();
 		}

CSSRenderer.php

@@ -32,7 +32,7 @@ class CSSRenderer {
 		}
 
 		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 @@ class CSSRenderer {
 	 *
 	 * @return string Rendered CSS
 	 */
-	function render() {
+	function render( $functionWhitelist = [], $propertyBlacklist = [] ) {
 
 		$css = '';
 
@@ -60,11 +60,29 @@ class CSSRenderer {
 				$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 .= "} ";

TemplateStyles.hooks.php

@@ -15,11 +15,20 @@ class TemplateStylesHooks {
 		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 ) {
 		$tree = gzdecode( $blob );
 		if ( $tree ) {
@@ -33,17 +42,12 @@ class TemplateStylesHooks {
 	}
 
 	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,20 +73,41 @@ class TemplateStylesHooks {
 
 		}
 
-		$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;
+			}
+		) );
+	}
+
 	/**
 	 * Parser hook for <templatestyles>.
 	 * If there is a CSS provided, render its source on the page and attach the
@@ -101,7 +126,10 @@ class TemplateStylesHooks {
 		$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 @@ class TemplateStylesHooks {
 			. Html::rawElement(
 				'p',
 				[ 'class' => 'mw-templatestyles-caption' ],
-				wfMessage( 'templatedata-doc-title' ) )
+				wfMessage( 'templatestyles-doc-title' ) )
 			. Html::element(
 				'pre',
 				[ 'class' => 'mw-templatestyles-stylesheet' ],

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"
 	}
 }