Contact Form: Wrap mail messages in HTML tags to avoid spam filters (#…

…6658) * Contact Form: Wrap mail messages in HTML tags to avoid spam filters This avoid's SpamAssassin's `HTML_MIME_NO_HTML_TAG` rule. See #6114 * Contact Form: Avoid wrapping in HTML if already has `<html>` tag A plugin could have added the tag via the `contact_form_message` filter, for instance. * Contact Form: Strip formatting tabs before injecting message This prevents the function from unintentionally stripping tabs that were intentionally added to the message body. * Contact Form: Generate all message components using HTML Previously some components were generated with HTML while others relied on plain-text processing, like hardcoded "\n" line breaks. This makes the function more consistent and logical. The unit test for line breaks is no longer relevant, because the body now uses HTML for formatting rather than "\n" breaks, and the plain-text alternative has processing to convert `<br>` breaks to "\n".
Automattic · Mar 28, 2017 · 7da53e8 · 7da53e8
1 parent a04b975
commit 7da53e8
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 8 deletions.
diff --git a/modules/contact-form/grunion-contact-form.php b/modules/contact-form/grunion-contact-form.php
@@ -2099,7 +2099,7 @@ function process_submission() {
 
 		array_push(
 			$message,
-			"", // Empty line left intentionally
+			"<br />",
 			'<hr />',
 			__( 'Time:', 'jetpack' ) . ' ' . $time . '<br />',
 			__( 'IP Address:', 'jetpack' ) . ' ' . $comment_author_IP . '<br />',
@@ -2109,18 +2109,18 @@ function process_submission() {
 		if ( is_user_logged_in() ) {
 			array_push(
 				$message,
-				'',
 				sprintf(
-					__( 'Sent by a verified %s user.', 'jetpack' ),
+					'<p>' . __( 'Sent by a verified %s user.', 'jetpack' ) . '</p>',
 					isset( $GLOBALS['current_site']->site_name ) && $GLOBALS['current_site']->site_name ?
 						$GLOBALS['current_site']->site_name : '"' . get_option( 'blogname' ) . '"'
 				)
 			);
 		} else {
-			array_push( $message, __( 'Sent by an unverified visitor to your site.', 'jetpack' ) );
+			array_push( $message, '<p>' . __( 'Sent by an unverified visitor to your site.', 'jetpack' ) . '</p>' );
 		}
 
-		$message = join( $message, "\n" );
+		$message = join( $message, '' );
+
 		/**
 		 * Filters the message sent via email after a successfull form submission.
 		 *
@@ -2132,6 +2132,9 @@ function process_submission() {
 		 */
 		$message = apply_filters( 'contact_form_message', $message );
 
+		// This is called after `contact_form_message`, in order to preserve back-compat
+		$message = self::wrap_message_in_html_tags( $message );
+
 		update_post_meta( $post_id, '_feedback_email', $this->addslashes_deep( compact( 'to', 'message' ) ) );
 
 		/**
@@ -2218,6 +2221,41 @@ function process_submission() {
 		exit;
 	}
 
+	/**
+	 * Wrap a message body with the appropriate in HTML tags
+	 *
+	 * This helps to ensure correct parsing by clients, and also helps avoid triggering spam filtering rules
+	 *
+	 * @param string $body
+	 *
+	 * @return string
+	 */
+	static function wrap_message_in_html_tags( $body ) {
+		// Don't do anything if the message was already wrapped in HTML tags
+		// That could have be done by a plugin via filters
+		if ( false !== strpos( $body, '<html' ) ) {
+			return $body;
+		}
+
+		$html_message = sprintf(
+			// The tabs are just here so that the raw code is correctly formatted for developers
+			// They're removed so that they don't affect the final message sent to users
+			str_replace( "\t", '',
+				"<!doctype html>
+				<html xmlns=\"http://www.w3.org/1999/xhtml\">
+				<body>
+
+				%s
+
+				</body>
+				</html>"
+			),
+			$body
+		);
+
+		return $html_message;
+	}
+
 	/**
 	 * Add a plain-text alternative part to an outbound email
 	 *
@@ -2227,7 +2265,17 @@ function process_submission() {
 	 * @param PHPMailer $phpmailer
 	 */
 	static function add_plain_text_alternative( $phpmailer ) {
-		$phpmailer->AltBody = strip_tags( $phpmailer->Body );
+		// Add an extra break so that the extra space above the <p> is preserved after the <p> is stripped out
+		$alt_body = str_replace( '<p>', '<p><br />', $phpmailer->Body );
+
+		// Convert <br> to \n breaks, to preserve the space between lines that we want to keep
+		$alt_body = str_replace( array( '<br>', '<br />' ), "\n", $alt_body );
+
+		// Convert <hr> to an plain-text equivalent, to preserve the integrity of the message
+		$alt_body = str_replace( array( "<hr>", "<hr />" ), "----\n", $alt_body );
+
+		// Trim the plain text message to remove the \n breaks that were after <doctype>, <html>, and <body>
+		$phpmailer->AltBody = trim( strip_tags( $alt_body ) );
 	}
 
 	function addslashes_deep( $value ) {

diff --git a/tests/php/modules/contact-form/test_class.grunion-contact-form.php b/tests/php/modules/contact-form/test_class.grunion-contact-form.php
@@ -331,8 +331,6 @@ public function pre_test_process_submission_sends_correct_single_email( $args ){
 		$this->assertContains( 'john@example.com', $args['to'] );
 		$this->assertEquals( 'Hello there!', $args['subject'] );
 
-		$this->assertContains( "<br /><br />\n", $args['message'], 'lines should be separated by newline characters' );
-
 		$expected = '<b>Name:</b> John Doe<br /><br />';
 		$expected .= '<b>Dropdown:</b> First option<br /><br />';
 		$expected .= '<b>Radio:</b> Second option<br /><br />';