From 0c2079421912cab59fafa8cf9d34ae66cc567680 Mon Sep 17 00:00:00 2001 From: Kijin Sung Date: Wed, 22 Feb 2017 20:58:37 +0900 Subject: [PATCH 1/8] Implement several template filters --- classes/template/TemplateHandler.class.php | 163 +++++++++++++++++++-- 1 file changed, 147 insertions(+), 16 deletions(-) diff --git a/classes/template/TemplateHandler.class.php b/classes/template/TemplateHandler.class.php index 1dd8364cf..7af89d0c9 100644 --- a/classes/template/TemplateHandler.class.php +++ b/classes/template/TemplateHandler.class.php @@ -554,26 +554,127 @@ class TemplateHandler } else { - $escape_option = $this->config->autoescape !== null ? 'auto' : 'noescape'; - if(preg_match('@^(.+)\\|((?:no)?escape)$@', $m[1], $mm)) - { - $m[1] = $mm[1]; - $escape_option = $mm[2]; - } - elseif($m[1] === '$content' && preg_match('@/layouts/.+/layout\.html$@', $this->file)) + // Get escape options. + if($m[1] === '$content' && preg_match('@/layouts/.+/layout\.html$@', $this->file)) { $escape_option = 'noescape'; } - $m[1] = self::_replaceVar($m[1]); - switch($escape_option) + else { - case 'auto': - return "config->autoescape === 'on' ? htmlspecialchars({$m[1]}, ENT_COMPAT, 'UTF-8', false) : {$m[1]}) ?>"; - case 'escape': - return ""; - case 'noescape': - return ""; + $escape_option = $this->config->autoescape !== null ? 'autoescape' : 'noescape'; } + + // Separate filters from variable. + if (preg_match('@^(.+?)(?_applyEscapeOption($var, $escape_option); + $var = "nl2br({$var})"; + $escape_option = 'noescape'; + break; + + case 'join': + $var = $filter_option ? "implode({$filter_option}, {$var})" : "implode(', ', {$var})"; + break; + + case 'date': + $var = $filter_option ? "getDisplayDateTime(ztime({$var}), {$filter_option})" : "getDisplayDateTime(ztime({$var}), 'Y-m-d H:i:s')"; + break; + + case 'format': + case 'number_format': + $var = $filter_option ? "number_format({$var}, {$filter_option})" : "number_format({$var})"; + break; + + case 'link': + $var = $this->_applyEscapeOption($var, $escape_option); + if ($filter_option) + { + $filter_option = $this->_applyEscapeOption($filter_option, $escape_option); + $var = "'' . {$var} . ''"; + } + else + { + $var = "'' . {$var} . ''"; + } + $escape_option = 'noescape'; + break; + + default: + $var = "INVALID FILTER ({$filter})"; + } + } + + // Apply the escape option and return. + return '_applyEscapeOption($var, $escape_option) . ' ?>'; } } @@ -773,6 +874,24 @@ class TemplateHandler return $m[0]; } + /** + * Apply escape option to an expression. + */ + private function _applyEscapeOption($str, $escape_option) + { + switch($escape_option) + { + case 'escape': + return "htmlspecialchars({$str}, ENT_COMPAT, 'UTF-8', true)"; + case 'noescape': + return "{$str}"; + case 'auto': + case 'autoescape': + default: + return "(\$this->config->autoescape === 'on' ? htmlspecialchars({$str}, ENT_COMPAT, 'UTF-8', false) : {$str})"; + } + } + /** * change relative path * @param string $path @@ -810,9 +929,21 @@ class TemplateHandler return $path; } + + /** + * Check if a string seems to contain a variable. + * + * @param string $str + * @return bool + */ + private static function _isVar($str) + { + return preg_match('@(?varname */ From 0c4dbc34ff885544903862a6f2d355ad0173c1f1 Mon Sep 17 00:00:00 2001 From: Kijin Sung Date: Wed, 22 Feb 2017 21:24:10 +0900 Subject: [PATCH 2/8] Add 'trim' filter and adjust some other settings --- classes/template/TemplateHandler.class.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/classes/template/TemplateHandler.class.php b/classes/template/TemplateHandler.class.php index 7af89d0c9..0807d60ed 100644 --- a/classes/template/TemplateHandler.class.php +++ b/classes/template/TemplateHandler.class.php @@ -623,6 +623,10 @@ class TemplateHandler $var = $filter_option ? "strip_tags({$var}, {$filter_option})" : "strip_tags({$var})"; break; + case 'trim': + $var = "trim({$var})"; + break; + case 'urlencode': $var = "rawurlencode({$var})"; break; @@ -669,7 +673,8 @@ class TemplateHandler break; default: - $var = "INVALID FILTER ({$filter})"; + $filter = escape_sqstr($filter); + $var = "'INVALID FILTER ({$filter})'"; } } From 7fd0f5df7b717cee3fd528be90f9836362a6259d Mon Sep 17 00:00:00 2001 From: Kijin Sung Date: Wed, 22 Feb 2017 21:24:26 +0900 Subject: [PATCH 3/8] Add unit tests for template filters --- tests/unit/classes/TemplateHandlerTest.php | 114 +++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/tests/unit/classes/TemplateHandlerTest.php b/tests/unit/classes/TemplateHandlerTest.php index 529c7230f..188ba4a09 100644 --- a/tests/unit/classes/TemplateHandlerTest.php +++ b/tests/unit/classes/TemplateHandlerTest.php @@ -300,6 +300,120 @@ class TemplateHandlerTest extends \Codeception\TestCase\Test array( '{\RX_BASEDIR}', '?>' + ), + // Rhymix autoescape + array( + '{$foo}', + PHP_EOL . '$this->config->autoescape = \'on\';' . "\n" . 'echo ($this->config->autoescape === \'on\' ? htmlspecialchars($__Context->foo, ENT_COMPAT, \'UTF-8\', false) : $__Context->foo) ?>' + ), + array( + '{$foo}', + PHP_EOL . '$this->config->autoescape = \'off\';' . "\n" . 'echo ($this->config->autoescape === \'on\' ? htmlspecialchars($__Context->foo, ENT_COMPAT, \'UTF-8\', false) : $__Context->foo) ?>' + ), + array( + '{$foo|auto}', + PHP_EOL . '$this->config->autoescape = \'on\';' . "\n" . 'echo ($this->config->autoescape === \'on\' ? htmlspecialchars($__Context->foo, ENT_COMPAT, \'UTF-8\', false) : $__Context->foo) ?>' + ), + array( + '{$foo|autoescape}', + PHP_EOL . '$this->config->autoescape = \'off\';' . "\n" . 'echo ($this->config->autoescape === \'on\' ? htmlspecialchars($__Context->foo, ENT_COMPAT, \'UTF-8\', false) : $__Context->foo) ?>' + ), + array( + '{$foo|escape}', + PHP_EOL . '$this->config->autoescape = \'on\';' . "\n" . 'echo htmlspecialchars($__Context->foo, ENT_COMPAT, \'UTF-8\', true) ?>' + ), + array( + '{$foo|escape}', + PHP_EOL . '$this->config->autoescape = \'off\';' . "\n" . 'echo htmlspecialchars($__Context->foo, ENT_COMPAT, \'UTF-8\', true) ?>' + ), + array( + '{$foo|noescape}', + PHP_EOL . '$this->config->autoescape = \'on\';' . "\n" . 'echo $__Context->foo ?>' + ), + array( + '{$foo|noescape}', + PHP_EOL . '$this->config->autoescape = \'off\';' . "\n" . 'echo $__Context->foo ?>' + ), + // Rhymix filters + array( + '

{$foo|escape}

', + '?>

foo, ENT_COMPAT, \'UTF-8\', true) ?>

' + ), + array( + '

{$foo|json}

', + '?>

foo) ?>

' + ), + array( + '

{$foo|urlencode}

', + '?>

foo) ?>

' + ), + array( + '

{$foo|lower|nl2br}

', + '?>

foo)) ?>

' + ), + array( + '

{$foo|join:/|upper}

', + '?>

foo)) ?>

' + ), + array( + '

{$foo|join:\||upper}

', + '?>

foo)) ?>

' + ), + array( + '

{$foo|join:$separator}

', + '?>

separator, $__Context->foo) ?>

' + ), + array( + '

{$foo|strip}

', + '?>

foo) ?>

' + ), + array( + '

{$foo|strip:
}

', + '?>

foo, \'
\') ?>

' + ), + array( + '

{$foo|strip:$mytags}

', + '?>

foo, $__Context->mytags) ?>

' + ), + array( + '

{$foo|strip:myfunc($mytags)}

', + '?>

foo, myfunc($__Context->mytags)) ?>

' + ), + array( + '

{$foo|trim|date}

', + '?>

foo)), \'Y-m-d H:i:s\') ?>

' + ), + array( + '

{$foo|date:His}

', + '?>

foo), \'His\') ?>

' + ), + array( + '

{$foo|format:2}

', + '?>

foo, \'2\') ?>

' + ), + array( + '

{$foo|date:His}

', + '?>

foo), \'His\') ?>

' + ), + array( + '

{$foo|link}

', + '?>

foo . \'">\' . $__Context->foo . \'\' ?>

' + ), + array( + '

{$foo|link:http://www.rhymix.org}

', + '?>

\' . $__Context->foo . \'\' ?>

' + ), + array( + '

{$foo|link:$url}

', + '?>

url . \'">\' . $__Context->foo . \'\' ?>

' + ), + array( + '

{$foo|link:$url}

', + PHP_EOL . '$this->config->autoescape = \'on\'; ?>

config->autoescape === \'on\' ? htmlspecialchars($__Context->url, ENT_COMPAT, \'UTF-8\', false) : $__Context->url) . \'">\' . ($this->config->autoescape === \'on\' ? htmlspecialchars($__Context->foo, ENT_COMPAT, \'UTF-8\', false) : $__Context->foo) . \'\' ?>

' + ), + array( + '

{$foo|dafuq}

', + '?>

' ), ); From 5638207fb021b95212f9b6d0c794b8f0c7a63fc0 Mon Sep 17 00:00:00 2001 From: Kijin Sung Date: Wed, 22 Feb 2017 21:29:15 +0900 Subject: [PATCH 4/8] Change behavior of 'autoescape' filter to always escape (but not double-escape) --- classes/template/TemplateHandler.class.php | 5 +++-- tests/unit/classes/TemplateHandlerTest.php | 10 +++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/classes/template/TemplateHandler.class.php b/classes/template/TemplateHandler.class.php index 0807d60ed..5a7a3e4b2 100644 --- a/classes/template/TemplateHandler.class.php +++ b/classes/template/TemplateHandler.class.php @@ -561,7 +561,7 @@ class TemplateHandler } else { - $escape_option = $this->config->autoescape !== null ? 'autoescape' : 'noescape'; + $escape_option = $this->config->autoescape !== null ? 'auto' : 'noescape'; } // Separate filters from variable. @@ -890,8 +890,9 @@ class TemplateHandler return "htmlspecialchars({$str}, ENT_COMPAT, 'UTF-8', true)"; case 'noescape': return "{$str}"; - case 'auto': case 'autoescape': + return "htmlspecialchars({$str}, ENT_COMPAT, 'UTF-8', false)"; + case 'auto': default: return "(\$this->config->autoescape === 'on' ? htmlspecialchars({$str}, ENT_COMPAT, 'UTF-8', false) : {$str})"; } diff --git a/tests/unit/classes/TemplateHandlerTest.php b/tests/unit/classes/TemplateHandlerTest.php index 188ba4a09..e84e45d7f 100644 --- a/tests/unit/classes/TemplateHandlerTest.php +++ b/tests/unit/classes/TemplateHandlerTest.php @@ -315,9 +315,17 @@ class TemplateHandlerTest extends \Codeception\TestCase\Test PHP_EOL . '$this->config->autoescape = \'on\';' . "\n" . 'echo ($this->config->autoescape === \'on\' ? htmlspecialchars($__Context->foo, ENT_COMPAT, \'UTF-8\', false) : $__Context->foo) ?>' ), array( - '{$foo|autoescape}', + '{$foo|auto}', PHP_EOL . '$this->config->autoescape = \'off\';' . "\n" . 'echo ($this->config->autoescape === \'on\' ? htmlspecialchars($__Context->foo, ENT_COMPAT, \'UTF-8\', false) : $__Context->foo) ?>' ), + array( + '{$foo|autoescape}', + PHP_EOL . '$this->config->autoescape = \'on\';' . "\n" . 'echo htmlspecialchars($__Context->foo, ENT_COMPAT, \'UTF-8\', false) ?>' + ), + array( + '{$foo|autoescape}', + PHP_EOL . '$this->config->autoescape = \'off\';' . "\n" . 'echo htmlspecialchars($__Context->foo, ENT_COMPAT, \'UTF-8\', false) ?>' + ), array( '{$foo|escape}', PHP_EOL . '$this->config->autoescape = \'on\';' . "\n" . 'echo htmlspecialchars($__Context->foo, ENT_COMPAT, \'UTF-8\', true) ?>' From d03c64d069602cfd8e0dd703321b81fecf0e4fb0 Mon Sep 17 00:00:00 2001 From: Kijin Sung Date: Thu, 23 Feb 2017 22:14:51 +0900 Subject: [PATCH 5/8] Make the test for filters more strict to prevent unintended parsing --- classes/template/TemplateHandler.class.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/classes/template/TemplateHandler.class.php b/classes/template/TemplateHandler.class.php index 5a7a3e4b2..f3ab187d3 100644 --- a/classes/template/TemplateHandler.class.php +++ b/classes/template/TemplateHandler.class.php @@ -565,7 +565,7 @@ class TemplateHandler } // Separate filters from variable. - if (preg_match('@^(.+?)(? Date: Thu, 23 Feb 2017 22:15:03 +0900 Subject: [PATCH 6/8] Add more unit tests for edge cases --- tests/unit/classes/TemplateHandlerTest.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/unit/classes/TemplateHandlerTest.php b/tests/unit/classes/TemplateHandlerTest.php index e84e45d7f..17550f586 100644 --- a/tests/unit/classes/TemplateHandlerTest.php +++ b/tests/unit/classes/TemplateHandlerTest.php @@ -423,6 +423,18 @@ class TemplateHandlerTest extends \Codeception\TestCase\Test '

{$foo|dafuq}

', '?>

' ), + array( + '

{$foo||$bar}

', + '?>

foo||$__Context->bar ?>

' + ), + array( + '

{htmlspecialchars($var, ENT_COMPAT|ENT_HTML401)}

', + '?>

var, ENT_COMPAT|ENT_HTML401) ?>

' + ), + array( + '

{$foo | $bar}

', + '?>

foo | $__Context->bar ?>

' + ), ); foreach ($tests as $test) From f338d385381e92ec3b28fb893ca83fdba0ad367e Mon Sep 17 00:00:00 2001 From: Kijin Sung Date: Thu, 23 Feb 2017 22:25:13 +0900 Subject: [PATCH 7/8] Improve regexp for template filters --- classes/template/TemplateHandler.class.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/classes/template/TemplateHandler.class.php b/classes/template/TemplateHandler.class.php index f3ab187d3..0105173f8 100644 --- a/classes/template/TemplateHandler.class.php +++ b/classes/template/TemplateHandler.class.php @@ -565,10 +565,10 @@ class TemplateHandler } // Separate filters from variable. - if (preg_match('@^(.+?)(? Date: Thu, 23 Feb 2017 22:25:25 +0900 Subject: [PATCH 8/8] Add even more unit tests for malformed filter detection --- tests/unit/classes/TemplateHandlerTest.php | 33 +++++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/tests/unit/classes/TemplateHandlerTest.php b/tests/unit/classes/TemplateHandlerTest.php index 17550f586..17a578e2c 100644 --- a/tests/unit/classes/TemplateHandlerTest.php +++ b/tests/unit/classes/TemplateHandlerTest.php @@ -419,17 +419,42 @@ class TemplateHandlerTest extends \Codeception\TestCase\Test '

{$foo|link:$url}

', PHP_EOL . '$this->config->autoescape = \'on\'; ?>

config->autoescape === \'on\' ? htmlspecialchars($__Context->url, ENT_COMPAT, \'UTF-8\', false) : $__Context->url) . \'">\' . ($this->config->autoescape === \'on\' ? htmlspecialchars($__Context->foo, ENT_COMPAT, \'UTF-8\', false) : $__Context->foo) . \'\' ?>

' ), + // Rhymix filters (reject malformed filters) array( '

{$foo|dafuq}

', '?>

' ), array( - '

{$foo||$bar}

', - '?>

foo||$__Context->bar ?>

' + '

{$foo|4}

', + '?>

foo|4 ?>

' ), array( - '

{htmlspecialchars($var, ENT_COMPAT|ENT_HTML401)}

', - '?>

var, ENT_COMPAT|ENT_HTML401) ?>

' + '

{$foo|a+7|lower}

', + '?>

foo|a+7) ?>

' + ), + array( + '

{$foo|Filter}

', + '?>

foo|Filter ?>

' + ), + array( + '

{$foo|filter++}

', + '?>

foo|filter++ ?>

' + ), + array( + '

{$foo|filter:}

', + '?>

foo|filter: ?>

' + ), + array( + '

{$foo|$bar}

', + '?>

foo|$__Context->bar ?>

' + ), + array( + '

{$foo||bar}

', + '?>

foo||bar ?>

' + ), + array( + '

{htmlspecialchars($var, ENT_COMPAT | ENT_HTML401)}

', + '?>

var, ENT_COMPAT | ENT_HTML401) ?>

' ), array( '

{$foo | $bar}

',