From 50aff07c9f257f8210e05d324a67cb819dfa9e96 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Fri, 12 Feb 2016 12:42:41 +0800 Subject: [PATCH 1/3] MDL-52136 core: Add support for quoting variables in mustache helpers This is required for when helpers include json-encoded variables as arguments. As an example, imagine a template with content: {{# str }} somekey, someidentifier, { "fullname": "{{ fullname }}" } {{/ str }} If the fullname variable were to include the double-quote character (e.g. John "Trevor" Doe) because of the way in which mustache renders content, it would become: {{# str }} somekey, someidentifier, { "fullname": "John "Trevor" Doe" } {{/ str }} This results in an invalid JSON structure. To work around this issue, the quote characters in the passed variable must be escaped: {{# str }} somekey, someidentifier, { "fullname": "John \"Trevor\" Doe" } {{/ str }} Unfortunately, Mustache provides no way of doing so natively. With this function, we can quote the text as appropriate: {{# str }} somekey, someidentifier, { "fullname": {{# quote }}{{ fullname }}{{/ quote }} } {{/ str }} This also handles the case where the quoted content includes the Mustache delimeter ({{ or }}). For example: fullname = 'John "}}Trevor{{" Doe' Ordinarily this would be rendered as: {{# str }} somekey, someidentifier, { "fullname": "John "}}Trevor{{" Doe" } {{/ str }} This rendering is both a JSON error, and also a mustache syntax error because of the mustache delimeters. The quote helper also escapes these by wrapping them in change delimeter tags: {{# str }} somekey, someidentifier, { "fullname": "John "{{=<% %>=}}}}<%={{ }}=%>Trevor{{=<% %>=}}{{{{=<% %>=}}" Doe" } {{/ str }} --- lib/amd/src/templates.js | 22 ++++++++ lib/classes/output/mustache_quote_helper.php | 55 +++++++++++++++++++ lib/classes/output/mustache_string_helper.php | 1 - lib/outputrenderers.php | 2 + 4 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 lib/classes/output/mustache_quote_helper.php diff --git a/lib/amd/src/templates.js b/lib/amd/src/templates.js index 6e4bc0c6e17..2204dc22e92 100644 --- a/lib/amd/src/templates.js +++ b/lib/amd/src/templates.js @@ -165,6 +165,27 @@ define([ 'core/mustache', return '{{_s' + index + '}}'; }; + /** + * Quote helper used to wrap content in quotes, and escape all quotes present in the content. + * + * @method quoteHelper + * @private + * @param {string} sectionText The text to parse the arguments from. + * @param {function} helper Used to render subsections of the text. + * @return {string} + */ + var quoteHelper = function(sectionText, helper) { + var content = helper(sectionText.trim(), this); + + // Escape the {{ and the ". + // This involves wrapping {{, and }} in change delimeter tags. + content = content + .replace('"', '\\"') + .replace(/([\{\}]{2,3})/g, '{{=<% %>=}}$1<%={{ }}=%>') + ; + return '"' + content + '"'; + }; + /** * Add some common helper functions to all context objects passed to templates. * These helpers match exactly the helpers available in php. @@ -182,6 +203,7 @@ define([ 'core/mustache', context.str = function() { return stringHelper; }; context.pix = function() { return pixHelper; }; context.js = function() { return jsHelper; }; + context.quote = function() { return quoteHelper; }; context.globals = { config : config }; context.currentTheme = themeName; }; diff --git a/lib/classes/output/mustache_quote_helper.php b/lib/classes/output/mustache_quote_helper.php new file mode 100644 index 00000000000..2f492fc8a7c --- /dev/null +++ b/lib/classes/output/mustache_quote_helper.php @@ -0,0 +1,55 @@ +. + +/** + * Wrap content in quotes, and escape all quotes used. + * + * @package core + * @category output + * @copyright 2016 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace core\output; + +/** + * Wrap content in quotes, and escape all quotes used. + * + * @copyright 2016 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class mustache_quote_helper { + + /** + * Wrap content in quotes, and escape all quotes used. + * + * Note: This helper is only compatible with the standard {{ }} delimeters. + * + * @param string $text The text to parse for arguments. + * @param Mustache_LambdaHelper $helper Used to render nested mustache variables. + * @return string + */ + public function quote($text, \Mustache_LambdaHelper $helper) { + // Split the text into an array of variables. + $content = trim($text); + $content = $helper->render($content); + + // Escape the {{ and the ". + $content = str_replace('"', '\\"', $content); + $content = preg_replace('([{}]{2,3})', '{{=<% %>=}}${0}<%={{ }}=%>', $content); + return '"' . $content . '"'; + } +} diff --git a/lib/classes/output/mustache_string_helper.php b/lib/classes/output/mustache_string_helper.php index 1b5d52a398d..0241be53059 100644 --- a/lib/classes/output/mustache_string_helper.php +++ b/lib/classes/output/mustache_string_helper.php @@ -76,4 +76,3 @@ class mustache_string_helper { return get_string($key, $component, $a); } } - diff --git a/lib/outputrenderers.php b/lib/outputrenderers.php index 99f4cb2fbfa..7b5f713a4b6 100644 --- a/lib/outputrenderers.php +++ b/lib/outputrenderers.php @@ -91,6 +91,7 @@ class renderer_base { $loader = new \core\output\mustache_filesystem_loader(); $stringhelper = new \core\output\mustache_string_helper(); + $quotehelper = new \core\output\mustache_quote_helper(); $jshelper = new \core\output\mustache_javascript_helper($this->page->requires); $pixhelper = new \core\output\mustache_pix_helper($this); @@ -99,6 +100,7 @@ class renderer_base { $helpers = array('config' => $safeconfig, 'str' => array($stringhelper, 'str'), + 'quote' => array($quotehelper, 'quote'), 'js' => array($jshelper, 'help'), 'pix' => array($pixhelper, 'pix')); From d64639a987b7ad428163667d78e4d52a7ff11705 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 12 Jan 2016 20:08:13 +0800 Subject: [PATCH 2/3] MDL-52136 mod_forum: Correctly quote get_string vars in mustache --- mod/forum/classes/output/forum_post.php | 58 ++++++++++++++++++- mod/forum/renderer.php | 2 +- .../forum_post_email_htmlemail.mustache | 7 ++- .../forum_post_email_textemail.mustache | 10 +++- ...m_post_emaildigestbasic_htmlemail.mustache | 6 +- ...m_post_emaildigestbasic_textemail.mustache | 5 +- ...um_post_emaildigestfull_htmlemail.mustache | 5 +- ...um_post_emaildigestfull_textemail.mustache | 5 +- mod/forum/tests/mail_test.php | 32 +++++----- 9 files changed, 101 insertions(+), 29 deletions(-) diff --git a/mod/forum/classes/output/forum_post.php b/mod/forum/classes/output/forum_post.php index 431dad61b2e..9f56eda7bf3 100644 --- a/mod/forum/classes/output/forum_post.php +++ b/mod/forum/classes/output/forum_post.php @@ -134,9 +134,65 @@ class forum_post implements \renderable { * Export this data so it can be used as the context for a mustache template. * * @param \mod_forum_renderer $renderer The render to be used for formatting the message and attachments + * @param bool $plaintext Whethe the target is a plaintext target * @return stdClass Data ready for use in a mustache template */ - public function export_for_template(\mod_forum_renderer $renderer) { + public function export_for_template(\mod_forum_renderer $renderer, $plaintext = false) { + if ($plaintext) { + return $this->export_for_template_text($renderer); + } else { + return $this->export_for_template_html($renderer); + } + } + + /** + * Export this data so it can be used as the context for a mustache template. + * + * @param \mod_forum_renderer $renderer The render to be used for formatting the message and attachments + * @return stdClass Data ready for use in a mustache template + */ + protected function export_for_template_text(\mod_forum_renderer $renderer) { + return array( + 'id' => html_entity_decode($this->post->id), + 'coursename' => html_entity_decode($this->get_coursename()), + 'courselink' => html_entity_decode($this->get_courselink()), + 'forumname' => html_entity_decode($this->get_forumname()), + 'showdiscussionname' => html_entity_decode($this->get_showdiscussionname()), + 'discussionname' => html_entity_decode($this->get_discussionname()), + 'subject' => html_entity_decode($this->get_subject()), + 'authorfullname' => html_entity_decode($this->get_author_fullname()), + 'postdate' => html_entity_decode($this->get_postdate()), + + // Format some components according to the renderer. + 'message' => html_entity_decode($renderer->format_message_text($this->cm, $this->post)), + 'attachments' => html_entity_decode($renderer->format_message_attachments($this->cm, $this->post)), + + 'canreply' => $this->canreply, + 'permalink' => $this->get_permalink(), + 'firstpost' => $this->get_is_firstpost(), + 'replylink' => $this->get_replylink(), + 'unsubscribediscussionlink' => $this->get_unsubscribediscussionlink(), + 'unsubscribeforumlink' => $this->get_unsubscribeforumlink(), + 'parentpostlink' => $this->get_parentpostlink(), + + 'forumindexlink' => $this->get_forumindexlink(), + 'forumviewlink' => $this->get_forumviewlink(), + 'discussionlink' => $this->get_discussionlink(), + + 'authorlink' => $this->get_authorlink(), + 'authorpicture' => $this->get_author_picture(), + + 'grouppicture' => $this->get_group_picture(), + ); + } + + /** + * Export this data so it can be used as the context for a mustache template. + * + * @param \mod_forum_renderer $renderer The render to be used for formatting the message and attachments + * @return stdClass Data ready for use in a mustache template + */ + protected function export_for_template_html(\mod_forum_renderer $renderer) { return array( 'id' => $this->post->id, 'coursename' => $this->get_coursename(), diff --git a/mod/forum/renderer.php b/mod/forum/renderer.php index ee2d4c58917..77e6b502a3c 100644 --- a/mod/forum/renderer.php +++ b/mod/forum/renderer.php @@ -191,7 +191,7 @@ class mod_forum_renderer extends plugin_renderer_base { * @return string */ public function render_forum_post_email(\mod_forum\output\forum_post_email $post) { - $data = $post->export_for_template($this); + $data = $post->export_for_template($this, $this->target === RENDERER_TARGET_TEXTEMAIL); return $this->render_from_template('mod_forum/' . $this->forum_post_template(), $data); } diff --git a/mod/forum/templates/forum_post_email_htmlemail.mustache b/mod/forum/templates/forum_post_email_htmlemail.mustache index f354b2333a7..9fa4c935f2e 100644 --- a/mod/forum/templates/forum_post_email_htmlemail.mustache +++ b/mod/forum/templates/forum_post_email_htmlemail.mustache @@ -102,8 +102,11 @@ {{{ subject }}}
- {{# str }} bynameondate, forum, { "name": "{{ authorfullname }}", "date": "{{ postdate }}" } {{/ str }} -
+ {{# str }} bynameondate, forum, { + "name": {{# quote }}{{ authorfullname }}{{/ quote }}, + "date": {{# quote }}{{ postdate }}{{/ quote }} + } {{/ str }} + diff --git a/mod/forum/templates/forum_post_email_textemail.mustache b/mod/forum/templates/forum_post_email_textemail.mustache index f3f61882a8c..63b6fce6b9b 100644 --- a/mod/forum/templates/forum_post_email_textemail.mustache +++ b/mod/forum/templates/forum_post_email_textemail.mustache @@ -45,14 +45,20 @@ {{{ coursename }}} -> {{# str }} forums, forum {{/ str }} -> {{{ forumname }}}{{# showdiscussionname }} -> {{{ discussionname }}} {{/ showdiscussionname }} {{ discussionlink }} {{{ subject }}} -{{# str }} bynameondate, forum, { "name": "{{{ authorfullname }}}", "date": "{{ postdate }}" } {{/ str }} +{{# str }} bynameondate, forum, { + "name": {{# quote }}{{{ authorfullname }}}{{/ quote }}, + "date": {{# quote}}{{ postdate }}{{/ quote }} + } {{/ str }} --------------------------------------------------------------------- {{{ message }}} {{{ attachments }}} --------------------------------------------------------------------- {{# canreply }} -{{# str }} postmailinfolink, forum, { "coursename": "{{{ coursename }}}", "replylink": "{{ replylink }}" } {{/ str }} +{{# str }} postmailinfolink, forum, { + "coursename": {{# quote }}{{{ coursename }}}{{/ quote }}, + "replylink": {{# quote }}{{ replylink }}{{/ quote }} + } {{/ str }} {{/ canreply }} {{# unsubscribeforumlink }} {{# str }} unsubscribelink, forum, {{{ unsubscribeforumlink }}} {{/ str }} diff --git a/mod/forum/templates/forum_post_emaildigestbasic_htmlemail.mustache b/mod/forum/templates/forum_post_emaildigestbasic_htmlemail.mustache index e36a9036343..2697d235aa0 100644 --- a/mod/forum/templates/forum_post_emaildigestbasic_htmlemail.mustache +++ b/mod/forum/templates/forum_post_emaildigestbasic_htmlemail.mustache @@ -44,7 +44,7 @@
{{{ subject }}} {{# str }} bynameondate, forum, { - "name": "{{ authorfullname }}", - "date": "{{ postdate }}" - } {{/ str }} + "name": {{# quote }}{{ authorfullname }}{{/ quote }}, + "date": {{# quote }}{{ postdate }}{{/ quote }} + } {{/ str }}
diff --git a/mod/forum/templates/forum_post_emaildigestbasic_textemail.mustache b/mod/forum/templates/forum_post_emaildigestbasic_textemail.mustache index 7f30cd91dd5..db9d242486d 100644 --- a/mod/forum/templates/forum_post_emaildigestbasic_textemail.mustache +++ b/mod/forum/templates/forum_post_emaildigestbasic_textemail.mustache @@ -33,5 +33,8 @@ }} {{ discussionlink }} -{{{ subject }}} {{# str }} bynameondate, forum, { "name": "{{{ authorfullname }}}", "date": "{{ postdate }}" } {{/ str }} +{{{ subject }}} {{# str }} bynameondate, forum, { + "name": {{# quote }}{{{ authorfullname }}}{{/ quote }}, + "date": {{# quote }}{{ postdate }}{{/ quote }} + } {{/ str }} --------------------------------------------------------------------- diff --git a/mod/forum/templates/forum_post_emaildigestfull_htmlemail.mustache b/mod/forum/templates/forum_post_emaildigestfull_htmlemail.mustache index 8cbd37fe9d0..2dc57799f37 100644 --- a/mod/forum/templates/forum_post_emaildigestfull_htmlemail.mustache +++ b/mod/forum/templates/forum_post_emaildigestfull_htmlemail.mustache @@ -87,7 +87,10 @@ {{{ subject }}}
- {{# str }} bynameondate, forum, { "name": "{{ authorfullname }}", "date": "{{ postdate }}" } {{/ str }} + {{# str }} bynameondate, forum, { + "name": {{# quote }}{{ authorfullname }}{{/ quote }}, + "date": {{# quote }}{{ postdate }}{{/ quote }} + } {{/ str }}
diff --git a/mod/forum/templates/forum_post_emaildigestfull_textemail.mustache b/mod/forum/templates/forum_post_emaildigestfull_textemail.mustache index deff33e490c..2cec6e7ebec 100644 --- a/mod/forum/templates/forum_post_emaildigestfull_textemail.mustache +++ b/mod/forum/templates/forum_post_emaildigestfull_textemail.mustache @@ -38,7 +38,10 @@ {{ discussionlink }} {{{ subject }}} ({{{ permalink }}}) -{{# str }} bynameondate, forum, { "name": "{{{ authorfullname }}}", "date": "{{ postdate }}" } {{/ str }} +{{# str }} bynameondate, forum, { + "name": {{# quote }}{{{ authorfullname }}}{{/ quote }}, + "date": {{# quote }}{{ postdate }}{{/ quote }} + } {{/ str }} --------------------------------------------------------------------- {{{ message }}} diff --git a/mod/forum/tests/mail_test.php b/mod/forum/tests/mail_test.php index f8f25e394c1..d979a4f271a 100644 --- a/mod/forum/tests/mail_test.php +++ b/mod/forum/tests/mail_test.php @@ -879,10 +879,8 @@ class mod_forum_mail_testcase extends advanced_testcase { // Single and double quotes everywhere. $newcase = $base; - $newcase['user']['lastname'] = 'Moodle\''; - // $newcase['user']['lastname'] = 'Moodle\'"'; // TODO: This breaks badly. See MDL-52136. - $newcase['course']['shortname'] = '101\''; - // $newcase['course']['shortname'] = '101\'"'; // TODO: This breaks badly. See MDL-52136. + $newcase['user']['lastname'] = 'Moodle\'"'; + $newcase['course']['shortname'] = '101\'"'; $newcase['forums'][0]['name'] = 'Moodle Forum\'"'; $newcase['forums'][0]['forumposts'][0]['name'] = 'Hello Moodle\'"'; $newcase['forums'][0]['forumposts'][0]['message'] = 'Welcome to Moodle\'"'; @@ -901,8 +899,8 @@ class mod_forum_mail_testcase extends advanced_testcase { $newcase['forums'][0]['forumposts'][0]['name'] = 'Hello Moodle>'; $newcase['forums'][0]['forumposts'][0]['message'] = 'Welcome to Moodle>'; $newcase['expectations'][0]['contents'] = array( - 'Attachment example.txt:', '~{\$a', '~&gt;', 'Love Moodle>', '101>', 'Moodle Forum>', - 'Hello Moodle>', 'Welcome to Moodle>'); + 'Attachment example.txt:', '~{\$a', '~&gt;', 'Love Moodle>', '101>', 'Moodle Forum>', + 'Hello Moodle>', 'Welcome to Moodle>'); $textcases['Text mail with gt and lt everywhere'] = array('data' => $newcase); // Ampersands everywhere. This case is completely borked because format_string() @@ -914,8 +912,8 @@ class mod_forum_mail_testcase extends advanced_testcase { $newcase['forums'][0]['forumposts'][0]['name'] = 'Hello Moodle&'; $newcase['forums'][0]['forumposts'][0]['message'] = 'Welcome to Moodle&'; $newcase['expectations'][0]['contents'] = array( - 'Attachment example.txt:', '~{\$a', '~&amp;', 'Love Moodle&', '101&', 'Moodle Forum&', - 'Hello Moodle&', 'Welcome to Moodle&'); + 'Attachment example.txt:', '~{\$a', '~&amp;', 'Love Moodle&', '101&', 'Moodle Forum&', + 'Hello Moodle&', 'Welcome to Moodle&'); $textcases['Text mail with ampersands everywhere'] = array('data' => $newcase); // Now the html cases. @@ -927,25 +925,23 @@ class mod_forum_mail_testcase extends advanced_testcase { $htmlbase['expectations'][0]['contents'] = array( '~{\$a', '~&(amp|lt|gt|quot|\#039);(?!course)', - '
( *\n *)?\n.*Hello Moodle', '>Moodle Forum', '>Welcome.*Moodle', '>Love Moodle', '>1\d1'); + '
( *\n *)?\n.*Hello Moodle', '>Moodle Forum', '>Welcome.*Moodle', '>Love Moodle', '>1\d1'); $htmlcases['HTML mail without ampersands, quotes or lt/gt'] = array('data' => $htmlbase); // Single and double quotes, lt and gt, ampersands everywhere. $newcase = $htmlbase; - $newcase['user']['lastname'] = 'Moodle\'>&'; - // $newcase['user']['lastname'] = 'Moodle\'">&'; // TODO: This breaks badly. See MDL-52136. - $newcase['course']['shortname'] = '101\'>&'; - // $newcase['course']['shortname'] = '101\'">&'; // TODO: This breaks badly. See MDL-52136. + $newcase['user']['lastname'] = 'Moodle\'">&'; + $newcase['course']['shortname'] = '101\'">&'; $newcase['forums'][0]['name'] = 'Moodle Forum\'">&'; $newcase['forums'][0]['forumposts'][0]['name'] = 'Hello Moodle\'">&'; $newcase['forums'][0]['forumposts'][0]['message'] = 'Welcome to Moodle\'">&'; $newcase['expectations'][0]['contents'] = array( '~{\$a', '~&(amp|lt|gt|quot|\#039);', - '
( *\n *)?\n.*Hello Moodle\'">&', '>Moodle Forum\'">&', - '>Welcome.*Moodle\'">&', '>Love Moodle&\#039;>&', '>1\d1\'>&'); + '
( *\n *)?\n.*Hello Moodle\'">&', '>Moodle Forum\'">&', + '>Welcome.*Moodle\'">&', '>Love Moodle&\#039;">&', '>101\'">&'); $htmlcases['HTML mail with quotes, gt, lt and ampersand everywhere'] = array('data' => $newcase); return $textcases + $htmlcases; @@ -1062,6 +1058,7 @@ class mod_forum_mail_testcase extends advanced_testcase { // If we have found the expectation and have contents to match, let's do it. if (isset($foundexpectation) and isset($foundexpectation['contents'])) { + $mail->body = quoted_printable_decode($mail->body); if (!is_array($foundexpectation['contents'])) { // Accept both string and array. $foundexpectation['contents'] = array($foundexpectation['contents']); } @@ -1069,6 +1066,7 @@ class mod_forum_mail_testcase extends advanced_testcase { if (strpos($content, '~') !== 0) { $this->assertRegexp('#' . $content . '#m', $mail->body); } else { + preg_match('#' . substr($content, 1) . '#m', $mail->body, $matches); $this->assertNotRegexp('#' . substr($content, 1) . '#m', $mail->body); } } From 0b1c48f422d7960c0e08d82f26f4a28721941531 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 2 Mar 2016 12:00:43 +0800 Subject: [PATCH 3/3] MDL-52136 mod_forum: forum_post is templatable --- mod/forum/classes/output/forum_post.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mod/forum/classes/output/forum_post.php b/mod/forum/classes/output/forum_post.php index 9f56eda7bf3..afb79bbb56b 100644 --- a/mod/forum/classes/output/forum_post.php +++ b/mod/forum/classes/output/forum_post.php @@ -34,7 +34,7 @@ defined('MOODLE_INTERNAL') || die(); * * @property boolean $viewfullnames Whether to override fullname() */ -class forum_post implements \renderable { +class forum_post implements \renderable, \templatable { /** * The course that the forum post is in. @@ -133,11 +133,11 @@ class forum_post implements \renderable { /** * Export this data so it can be used as the context for a mustache template. * - * @param \mod_forum_renderer $renderer The render to be used for formatting the message and attachments + * @param \base_renderer $renderer The render to be used for formatting the message and attachments * @param bool $plaintext Whethe the target is a plaintext target * @return stdClass Data ready for use in a mustache template */ - public function export_for_template(\mod_forum_renderer $renderer, $plaintext = false) { + public function export_for_template(\renderer_base $renderer, $plaintext = false) { if ($plaintext) { return $this->export_for_template_text($renderer); } else {