From 5d8ac32bcb73bb10308d24fe9d26abdc1c0714fe Mon Sep 17 00:00:00 2001 From: Nils Larsen Date: Thu, 28 Dec 2023 10:24:44 +0100 Subject: [PATCH 1/7] make commonsbooking_parse_template() customizable with respect to sanitazion to allow for HTML-sanitazion (default) or email subject (sanitize_text_field). Should fix issue 1433 for e-mail subjects with special characters like & --- includes/TemplateParser.php | 22 ++++++++++++---------- src/CB/CB.php | 8 ++++---- src/Messages/Message.php | 2 +- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/includes/TemplateParser.php b/includes/TemplateParser.php index 78f54c2d4..fc616bdcf 100644 --- a/includes/TemplateParser.php +++ b/includes/TemplateParser.php @@ -10,11 +10,11 @@ * * @return mixed */ -function commonsbooking_parse_template( string $template = '', $objects = [] ) { +function commonsbooking_parse_template( string $template = '', $objects = [], $sanitizeFunction = 'commonsbooking_sanitizeHTML' ) { $template = preg_replace_callback( '/\{{.*?\}}/', - function ( $match ) use ( $objects ) { - return commonsbooking_parse_template_callback( $match, $objects ); + function ( $match ) use ( $objects, $sanitizeFunction ) { + return commonsbooking_parse_template_callback( $match, $objects, $sanitizeFunction ); }, $template ); @@ -24,13 +24,13 @@ function ( $match ) use ( $objects ) { if ( preg_match_all( '/{{.*?}}/', $template ) === 0 ) { return apply_filters( 'commonsbooking_template_tag', $template ); } else { - return commonsbooking_parse_template( $template, $objects ); + return commonsbooking_parse_template( $template, $objects, $sanitizeFunction ); } } -function commonsbooking_parse_shortcode( $tag ) { +function commonsbooking_parse_shortcode( $tag, $sanitizeFunction = 'commonsbooking_sanitizeHTML' ) { $tag = (array) $tag; - return commonsbooking_parse_template_callback( $tag ); + return commonsbooking_parse_template_callback( $tag, [], $sanitizeFunction ); } /** @@ -44,21 +44,23 @@ function commonsbooking_parse_shortcode( $tag ) { * * @return false|mixed */ -function commonsbooking_parse_template_callback( $match, array $objects = [] ) { +function commonsbooking_parse_template_callback( $match, array $objects = [], $sanitizeFunction ) { if ( isset( $match[0] ) ) { $match = $match[0]; // extract the html before part, looking for {{[*] pattern if ( preg_match( '/\{\{\[([^\]]*)\]/m', $match, $html_before ) === 1 ) { - $html_before = commonsbooking_sanitizeHTML( preg_replace( '/(\{\{)|(\}\})|(\[)|(\])/m', '', $html_before[1] ) ); + $html_before = preg_replace( '/(\{\{)|(\}\})|(\[)|(\])/m', '', $html_before[1] ); + $html_before = call_user_func( $sanitizeFunction, $html_before ); } else { $html_before = ''; } // extract the html after part looking for [*]}} pattern if ( preg_match( '/\[([^\]]*)\]\}\}/m', $match, $html_after ) === 1 ) { - $html_after = commonsbooking_sanitizeHTML( preg_replace( '/(\{\{)|(\}\})|(\[)|(\])/m', '', $html_after[1] ) ); + $html_after = preg_replace( '/(\{\{)|(\}\})|(\[)|(\])/m', '', $html_after[1] ); + $html_after = call_user_func( $sanitizeFunction, $html_after ); } else { $html_after = ''; } @@ -81,7 +83,7 @@ function commonsbooking_parse_template_callback( $match, array $objects = [] ) { $post = $objects[ $path[0] ]; } - $rendered_template_tag = CB::get( commonsbooking_getCBType( $path[0] ), $path[1], $post ); + $rendered_template_tag = CB::get( commonsbooking_getCBType( $path[0] ), $path[1], $post, null, $sanitizeFunction ); if ( $rendered_template_tag !== null && strlen( $rendered_template_tag ) > 0 ) { return $html_before . $rendered_template_tag . $html_after; } else { diff --git a/src/CB/CB.php b/src/CB/CB.php index 8e8044488..4da86bb4f 100644 --- a/src/CB/CB.php +++ b/src/CB/CB.php @@ -29,7 +29,7 @@ public static function getInternalDateFormat(): string { * @return mixed * @throws Exception */ - public static function get( $key, $property, $wpObject = null, $args = null ) { + public static function get( $key, $property, $wpObject = null, $args = null, $sanitizeFunction = "commonsbooking_sanitizeHTML" ) { // Only CustomPost, WP_User or WP_Post ist allowed. if ( $wpObject && ! ( @@ -49,7 +49,7 @@ public static function get( $key, $property, $wpObject = null, $args = null ) { // If possible cast to CB Custom Post Type Model to get additional functions $wpObject = Helper::castToCBCustomType($wpObject, $key); - $result = self::lookUp( $key, $property, $wpObject, $args ); // Find matching methods, properties or metadata + $result = self::lookUp( $key, $property, $wpObject, $args, $sanitizeFunction ); // Find matching methods, properties or metadata $filterName = sprintf( 'commonsbooking_tag_%s_%s', $key, $property ); return apply_filters( $filterName, $result ); @@ -106,7 +106,7 @@ private static function getPostId( string $key ): ?int { * @return string|null * @throws Exception */ - public static function lookUp( string $key, string $property, $post, $args ): ?string { + public static function lookUp( string $key, string $property, $post, $args, $sanitizeFunction ): ?string { // in any case we need the post object, otherwise we cannot return anything if ( ! $post ) { return null; @@ -120,7 +120,7 @@ public static function lookUp( string $key, string $property, $post, $args ): ?s if ( $result ) { // sanitize output - return commonsbooking_sanitizeHTML( $result ); + return call_user_func( $sanitizeFunction, $result ); } return $result; diff --git a/src/Messages/Message.php b/src/Messages/Message.php index 9d5aed0a0..c72790475 100644 --- a/src/Messages/Message.php +++ b/src/Messages/Message.php @@ -116,7 +116,7 @@ protected function prepareMail( // parse templates & replaces template tags (e.g. {{item:name}}) $this->body = commonsbooking_sanitizeHTML( commonsbooking_parse_template( $template_body, $objects ) ); - $this->subject = sanitize_text_field( commonsbooking_parse_template( $template_subject, $objects ) ); + $this->subject = sanitize_text_field( commonsbooking_parse_template( $template_subject, $objects, "sanitize_text_field" ) ); // Setup mime type $this->headers[] = "MIME-Version: 1.0"; From 1dfe83f289a6cf22f19bb6e9b7ab8ddd13bd2ba0 Mon Sep 17 00:00:00 2001 From: Nils Larsen Date: Thu, 28 Dec 2023 12:20:54 +0100 Subject: [PATCH 2/7] update test of internal library function, because the functions now requires a fifth argument --- tests/php/CB/CBTest.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/php/CB/CBTest.php b/tests/php/CB/CBTest.php index ace3f92cb..f25686be6 100644 --- a/tests/php/CB/CBTest.php +++ b/tests/php/CB/CBTest.php @@ -37,19 +37,19 @@ class CBTest extends CustomPostTypeTest { public function testLookUp() { // Test if user meta value is found when handing over WP_Post object $post = get_post( $this->postInstanceId ); - $this->assertEquals( CB::lookUp( 'user', $this->userMetaKey, $post, [] ), $this->userMetaValue ); + $this->assertEquals( CB::lookUp( 'user', $this->userMetaKey, $post, [], 'commonsbooking_sanitizeHTML' ), $this->userMetaValue ); // Test if post title is returned when handing over post key and post object - $this->assertEquals( CB::lookUp( 'post', 'post_title', $post, [] ), $this->postTitle ); + $this->assertEquals( CB::lookUp( 'post', 'post_title', $post, [], 'commonsbooking_sanitizeHTML' ), $this->postTitle ); // Test if null is returned when trying to get not existing property of post - $this->assertEquals( null, CB::lookUp( 'user', 'post_title', $post, [] ) ); + $this->assertEquals( null, CB::lookUp( 'user', 'post_title', $post, [], 'commonsbooking_sanitizeHTML' ) ); // Trying to get property without post object - $this->assertEquals( CB::lookUp( 'user', 'test', null, [] ), null ); - $this->assertEquals( CB::lookUp( 'booking', 'test', null, [] ), null ); - $this->assertEquals( CB::lookUp( 'item', 'test', null, [] ), null ); - $this->assertEquals( CB::lookUp( 'location', 'test', null, [] ), null ); + $this->assertEquals( CB::lookUp( 'user', 'test', null, [], 'commonsbooking_sanitizeHTML' ), null ); + $this->assertEquals( CB::lookUp( 'booking', 'test', null, [], 'commonsbooking_sanitizeHTML' ), null ); + $this->assertEquals( CB::lookUp( 'item', 'test', null, [], 'commonsbooking_sanitizeHTML' ), null ); + $this->assertEquals( CB::lookUp( 'location', 'test', null, [], 'commonsbooking_sanitizeHTML' ), null ); } public function testGet() { From ae9a0523e5f22c7adeda4109dde2007b235a2967 Mon Sep 17 00:00:00 2001 From: Nils Larsen Date: Fri, 29 Dec 2023 09:25:43 +0100 Subject: [PATCH 3/7] add documentation --- includes/TemplateParser.php | 2 ++ src/CB/CB.php | 4 +++- src/Messages/Message.php | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/includes/TemplateParser.php b/includes/TemplateParser.php index fc616bdcf..4be61c5c6 100644 --- a/includes/TemplateParser.php +++ b/includes/TemplateParser.php @@ -7,6 +7,7 @@ * * @param string $template * @param array $objects + * @param callable $sanitizeFunction The callable used to remove unwanted tags/characters (use default 'commonsbooking_sanitizeHTML' or 'sanitize_text_field') * * @return mixed */ @@ -41,6 +42,7 @@ function commonsbooking_parse_shortcode( $tag, $sanitizeFunction = 'commonsbooki * * @param mixed $match * @param array $objects + * @param callable $sanitizeFunction The callable used to remove unwanted tags/characters * * @return false|mixed */ diff --git a/src/CB/CB.php b/src/CB/CB.php index 4da86bb4f..20fd4ff40 100644 --- a/src/CB/CB.php +++ b/src/CB/CB.php @@ -25,8 +25,9 @@ public static function getInternalDateFormat(): string { * @param mixed $property * @param \WP_Post|WP_User $wpObject * @param mixed $args + * @param callable $sanitizeFunction The callable used to remove unwanted tags/characters (use default 'commonsbooking_sanitizeHTML' or 'sanitize_text_field') * - * @return mixed + * @return Property of (custom) post (sanitized) or null if not found * @throws Exception */ public static function get( $key, $property, $wpObject = null, $args = null, $sanitizeFunction = "commonsbooking_sanitizeHTML" ) { @@ -102,6 +103,7 @@ private static function getPostId( string $key ): ?int { * @param string $property * @param $post * @param $args + * @param callable $sanitizeFunction The callable used to remove unwanted tags/characters * * @return string|null * @throws Exception diff --git a/src/Messages/Message.php b/src/Messages/Message.php index c72790475..3ad00fb25 100644 --- a/src/Messages/Message.php +++ b/src/Messages/Message.php @@ -115,6 +115,8 @@ protected function prepareMail( } // parse templates & replaces template tags (e.g. {{item:name}}) + // 'body' is HTML. 'subject' is not HTML needs alternative sanitation such that characters like & + // do not get converted to HTML-entities like & $this->body = commonsbooking_sanitizeHTML( commonsbooking_parse_template( $template_body, $objects ) ); $this->subject = sanitize_text_field( commonsbooking_parse_template( $template_subject, $objects, "sanitize_text_field" ) ); From 0e7fa9512739759fac80b820cd15870765f1c801 Mon Sep 17 00:00:00 2001 From: Nils Larsen Date: Fri, 29 Dec 2023 09:27:44 +0100 Subject: [PATCH 4/7] as shortcodes are ALWAYS html, revert the added parameter to commonsbooking_parse_shortcode(), and rather always use commonsbooking_sanitizeHTML for sanitation --- includes/TemplateParser.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/TemplateParser.php b/includes/TemplateParser.php index 4be61c5c6..2d1f03a0b 100644 --- a/includes/TemplateParser.php +++ b/includes/TemplateParser.php @@ -29,9 +29,9 @@ function ( $match ) use ( $objects, $sanitizeFunction ) { } } -function commonsbooking_parse_shortcode( $tag, $sanitizeFunction = 'commonsbooking_sanitizeHTML' ) { +function commonsbooking_parse_shortcode( $tag ) { $tag = (array) $tag; - return commonsbooking_parse_template_callback( $tag, [], $sanitizeFunction ); + return commonsbooking_parse_template_callback( $tag, [], 'commonsbooking_sanitizeHTML' ); } /** From eac7ad95799f5b44b36eadd691d31288570d5947 Mon Sep 17 00:00:00 2001 From: Nils Larsen Date: Fri, 29 Dec 2023 09:33:16 +0100 Subject: [PATCH 5/7] remove parameter to filter which was very likely added by mistake (in recent commit 87bb3b226) --- src/Messages/Message.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Messages/Message.php b/src/Messages/Message.php index 3ad00fb25..673af2b6e 100644 --- a/src/Messages/Message.php +++ b/src/Messages/Message.php @@ -147,7 +147,7 @@ protected function prepareMail( */ public function SendNotificationMail() { $to = apply_filters( 'commonsbooking_mail_to', $this->to, $this->action ); - $subject = apply_filters( 'commonsbooking_mail_subject', $this->subject, $this->action, 'sanitize_text_field' ); + $subject = apply_filters( 'commonsbooking_mail_subject', $this->subject, $this->action ); $body = apply_filters( 'commonsbooking_mail_body', $this->body, $this->action ); $attachment = apply_filters( 'commonsbooking_mail_attachment', $this->attachment, $this->action); $headers = implode( "\r\n", $this->headers ); From 49c7f6bba5699c3afaf3291d71b6fb68a170aac5 Mon Sep 17 00:00:00 2001 From: Nils Larsen Date: Wed, 3 Jan 2024 21:56:57 +0100 Subject: [PATCH 6/7] make commonsbooking_sanitizeHTML() the default sanitation function for commonsbooking_parse_template_callback() --- includes/TemplateParser.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/TemplateParser.php b/includes/TemplateParser.php index 2d1f03a0b..bcebd5049 100644 --- a/includes/TemplateParser.php +++ b/includes/TemplateParser.php @@ -31,7 +31,7 @@ function ( $match ) use ( $objects, $sanitizeFunction ) { function commonsbooking_parse_shortcode( $tag ) { $tag = (array) $tag; - return commonsbooking_parse_template_callback( $tag, [], 'commonsbooking_sanitizeHTML' ); + return commonsbooking_parse_template_callback( $tag ); } /** @@ -46,7 +46,7 @@ function commonsbooking_parse_shortcode( $tag ) { * * @return false|mixed */ -function commonsbooking_parse_template_callback( $match, array $objects = [], $sanitizeFunction ) { +function commonsbooking_parse_template_callback( $match, array $objects = [], $sanitizeFunction = 'commonsbooking_sanitizeHTML' ) { if ( isset( $match[0] ) ) { $match = $match[0]; From ae2f9dd97aa4854a098d249f7809aeb8a2503f2c Mon Sep 17 00:00:00 2001 From: Hans Morbach <6433480+hansmorb@users.noreply.github.com> Date: Thu, 4 Jan 2024 12:16:22 -0500 Subject: [PATCH 7/7] re-added changes after merge --- src/Messages/Message.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Messages/Message.php b/src/Messages/Message.php index a641b0c7f..deaa0c541 100644 --- a/src/Messages/Message.php +++ b/src/Messages/Message.php @@ -90,7 +90,7 @@ public function getHeaders() { } public function getSubject() { - return apply_filters( 'commonsbooking_mail_subject', $this->subject, $this->getAction(), 'sanitize_text_field' ); + return apply_filters( 'commonsbooking_mail_subject', $this->subject, $this->getAction() ); } public function getBody() {