Skip to content

Fix handling of BODY styles in HTML messages (#9911) #9923

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
alecpl wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Fix handling of BODY styles in HTML messages (#9911) #9923

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4ea8116
Fix handling of BODY styles in HTML messages (#9911)
alecpl Jul 6, 2025
b576dc7
Fix tests
alecpl Jul 6, 2025
dd3aac2
Fixes
alecpl Jul 7, 2025
14fd3cf
Fix
alecpl Jul 7, 2025
a0e3739
Fix test regression
alecpl Jul 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions program/actions/mail/compose.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -989,22 +989,17 @@ public static function prepare_html_body($body, $wash_params = [])
{
static $part_no;

// Set attributes of the part container
$container_id = self::$COMPOSE['mode'] . 'body' . (++$part_no);

$wash_params += [
'safe' => self::$MESSAGE->is_safe,
'css_prefix' => 'v' . $part_no,
'safe' => self::$MESSAGE && self::$MESSAGE->is_safe,
'css_prefix' => 'v' . (++$part_no),
'add_comments' => false,
];

if (self::$COMPOSE['mode'] == rcmail_sendmail::MODE_DRAFT) {
if (self::$COMPOSE && self::$COMPOSE['mode'] == rcmail_sendmail::MODE_DRAFT) {
// convert TinyMCE's empty-line sequence (#1490463)
$body = preg_replace('/<p>\xC2\xA0<\/p>/', '<p><br /></p>', $body);
// remove <body> tags (not their content)
$wash_params['ignore_elements'] = ['body'];
} else {
$wash_params['container_id'] = $container_id;
}

// Make the HTML content safe and clean
Expand Down
40 changes: 20 additions & 20 deletions program/actions/mail/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ public static function wash_html($html, $p, $cid_replaces = [])
'css_prefix' => $p['css_prefix'],
// internal configuration
'container_id' => $p['container_id'],
'body_class' => $p['body_class'] ?? '',
'body_class' => $p['body_class'] ?? 'rcmBody',
];

if (empty($p['inline_html'])) {
Expand All @@ -974,14 +974,7 @@ public static function wash_html($html, $p, $cid_replaces = [])

if (!empty($p['inline_html'])) {
$washer->add_callback('body', 'rcmail_action_mail_index::washtml_callback');

if ($wash_opts['body_class']) {
self::$wash_html_body_attrs['class'] = $wash_opts['body_class'];
}

if ($wash_opts['container_id']) {
self::$wash_html_body_attrs['id'] = $wash_opts['container_id'];
}
self::$wash_html_body_attrs['class'] = $wash_opts['body_class'];
}

if (empty($p['skip_washer_form_callback'])) {
Expand Down Expand Up @@ -1149,11 +1142,10 @@ public static function washtml_callback($tagname, $attrib, $content, $washtml)
if (strlen($out)) {
$css_prefix = $washtml->get_config('css_prefix');
$is_safe = $washtml->get_config('allow_remote');
$body_class = $washtml->get_config('body_class') ?: '';
$body_class = $washtml->get_config('body_class');
$cont_id = $washtml->get_config('container_id') ?: '';
$cont_id = trim($cont_id . ($body_class ? " div.{$body_class}" : ''));

$out = rcube_utils::mod_css_styles($out, $cont_id, $is_safe, $css_prefix);
$out = rcube_utils::mod_css_styles($out, $cont_id, $is_safe, $css_prefix, $body_class);

$out = html::tag('style', ['type' => 'text/css'], $out);
}
Expand Down Expand Up @@ -1183,11 +1175,20 @@ public static function washtml_callback($tagname, $attrib, $content, $washtml)
$style['background-image'] = "url({$value})";
}
break;
case 'class':
if (!empty($value)) {
$attrs['class'] = trim(($attrs['class'] ?? '') . ' ' . $value);
}
break;
default:
$attrs[$attr_name] = $value;
}
}

if (isset($attrs['class']) && empty($attrs['class'])) {
unset($attrs['class']);
}

if (!empty($style)) {
foreach ($style as $idx => $val) {
$style[$idx] = $idx . ': ' . $val;
Expand Down Expand Up @@ -1282,14 +1283,13 @@ public static function washtml_link_callback($tag, $attribs, $content, $washtml)

if ($tag == 'link' && preg_match('/^https?:\/\//i', $attrib['href'])) {
$tempurl = 'tmp-' . md5($attrib['href']) . '.css';
$_SESSION['modcssurls'][$tempurl] = $attrib['href'];
$attrib['href'] = $rcmail->url([
'task' => 'utils',
'action' => 'modcss',
'u' => $tempurl,
'c' => $washtml->get_config('container_id'),
'p' => $washtml->get_config('css_prefix'),
]);
$_SESSION['modcssurls'][$tempurl] = [
'url' => $attrib['href'],
'container_id' => $washtml->get_config('container_id'),
'css_prefix' => $washtml->get_config('css_prefix'),
'body_class' => $washtml->get_config('body_class'),
];
$attrib['href'] = $rcmail->url(['task' => 'utils', 'action' => 'modcss', 'u' => $tempurl]);
$content = null;
} elseif (preg_match('/^mailto:(.+)/i', $attrib['href'], $mailto)) {
$url_parts = explode('?', html_entity_decode($mailto[1], \ENT_QUOTES, 'UTF-8'), 2);
Expand Down
65 changes: 38 additions & 27 deletions program/actions/mail/show.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,6 @@ public static function message_body($attrib)
$rcmail = rcmail::get_instance();
$safe_mode = self::$MESSAGE->is_safe || !empty($_GET['_safe']);
$out = '';
$part_no = 0;

$header_attrib = [];
foreach ($attrib as $attr => $value) {
Expand Down Expand Up @@ -692,32 +691,8 @@ public static function message_body($attrib)
self::message_error();
}

$plugin = $rcmail->plugins->exec_hook('message_body_prefix',
['part' => $part, 'prefix' => '', 'message' => self::$MESSAGE]);

// Set attributes of the part container
$container_class = $part->ctype_secondary == 'html' ? 'message-htmlpart' : 'message-part';
$container_id = $container_class . (++$part_no);
$container_attrib = ['class' => $container_class, 'id' => $container_id];

$body_args = [
'safe' => $safe_mode,
'plain' => !$rcmail->config->get('prefer_html'),
'css_prefix' => 'v' . $part_no,
'body_class' => 'rcmBody',
'container_id' => $container_id,
'container_attrib' => $container_attrib,
];

// Parse the part content for display
$body = self::print_body($body, $part, $body_args);

// check if the message body is PGP encrypted
if (strpos($body, '-----BEGIN PGP MESSAGE-----') !== false) {
$rcmail->output->set_env('is_pgp_content', '#' . $container_id);
}

$out .= html::div($body_args['container_attrib'], $plugin['prefix'] . $body);
// Process the part content for display
$out .= self::prepare_part_body($body, $part);
}
}
} else {
Expand Down Expand Up @@ -888,4 +863,40 @@ public static function mdn_request_handler($message)
}
}
}

/**
* Prepare message part content for display
*/
protected static function prepare_part_body($body, $part)
{
static $part_no;

$rcmail = rcmail::get_instance();

$plugin = $rcmail->plugins->exec_hook('message_body_prefix',
['part' => $part, 'prefix' => '', 'message' => self::$MESSAGE]);

// Set attributes of the part container
$container_class = $part->ctype_secondary == 'html' ? 'message-htmlpart' : 'message-part';
$container_id = $container_class . (++$part_no);
$container_attrib = ['class' => $container_class, 'id' => $container_id];

$body_args = [
'safe' => (self::$MESSAGE && self::$MESSAGE->is_safe) || !empty($_GET['_safe']),
'plain' => !$rcmail->config->get('prefer_html'),
'css_prefix' => 'v' . $part_no,
'container_id' => $container_id,
'container_attrib' => $container_attrib,
];

// Parse the part content for display
$body = self::print_body($body, $part, $body_args);

// check if the message body is PGP encrypted
if (strpos($body, '-----BEGIN PGP MESSAGE-----') !== false) {
$rcmail->output->set_env('is_pgp_content', '#' . $body_args['container_id']);
}

return html::div($body_args['container_attrib'], $plugin['prefix'] . $body);
}
}
16 changes: 7 additions & 9 deletions program/actions/utils/modcss.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ public function run($args = [])
$rcmail->output->sendExitError(403, 'Unauthorized request');
}

$realurl = $_SESSION['modcssurls'][$url];
$data = $_SESSION['modcssurls'][$url];
$realurl = $data['url'] ?? '';

// don't allow any other connections than http(s)
if (!preg_match('~^https?://~i', $realurl, $matches)) {
Expand All @@ -59,16 +60,13 @@ public function run($args = [])
rcube::raise_error($e, true, false);
}

$cid = rcube_utils::get_input_string('_c', rcube_utils::INPUT_GET);
$prefix = rcube_utils::get_input_string('_p', rcube_utils::INPUT_GET);
if ($source !== false && $ctype && preg_match('~^text/(css|plain)~i', $ctype)) {
$container_id = $data['container_id'] ?? '';
$css_prefix = $data['css_prefix'] ?? '';
$body_class = $data['body_class'] ?? '';

$container_id = preg_replace('/[^a-z0-9]/i', '', $cid);
$css_prefix = preg_replace('/[^a-z0-9]/i', '', $prefix);
$ctype_regexp = '~^text/(css|plain)~i';

if ($source !== false && $ctype && preg_match($ctype_regexp, $ctype)) {
$rcmail->output->sendExit(
rcube_utils::mod_css_styles($source, $container_id, false, $css_prefix),
rcube_utils::mod_css_styles($source, $container_id, false, $css_prefix, $body_class),
['Content-Type: text/css']
);
}
Expand Down
14 changes: 6 additions & 8 deletions program/lib/Roundcube/rcube_utils.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,10 +422,11 @@ public static function html_identifier($str, $encode = false)
* @param string $container_id Container ID to use as prefix
* @param bool $allow_remote Allow remote content
* @param string $prefix Prefix to be added to id/class identifier
* @param string $body_class Optional class name for the used-to-be-body element
*
* @return string Modified CSS source
*/
public static function mod_css_styles($source, $container_id, $allow_remote = false, $prefix = '')
public static function mod_css_styles($source, $container_id, $allow_remote = false, $prefix = '', $body_class = '')
{
$source = self::xss_entity_decode($source);

Expand Down Expand Up @@ -485,7 +486,7 @@ public static function mod_css_styles($source, $container_id, $allow_remote = fa
// for cases like @media { body { position: fixed; } } (#5811)
$excl = '(?!' . substr($replacements->pattern, 1, -1) . ')';
$regexp = '/(^\s*|,\s*|\}\s*|\{\s*)(' . $excl . ':?[a-z0-9\._#\*\[][a-z0-9\._:\(\)#=~ \[\]"\|\>\+\$\^-]*)/im';
$callback = static function ($matches) use ($container_id, $prefix) {
$callback = static function ($matches) use ($container_id, $prefix, $body_class) {
$replace = $matches[2];

if (stripos($replace, ':root') === 0) {
Expand All @@ -497,6 +498,9 @@ public static function mod_css_styles($source, $container_id, $allow_remote = fa
}

if ($container_id) {
// replace body definition because we stripped off the <body> tag
$replace = preg_replace('/^\s*body/i', $body_class ? ".{$body_class}" : '', $replace);

$replace = "#{$container_id} " . $replace;
}

Expand All @@ -509,12 +513,6 @@ public static function mod_css_styles($source, $container_id, $allow_remote = fa
$source = preg_replace_callback($regexp, $callback, $source);
}

// replace body definition because we also stripped off the <body> tag
if ($container_id) {
$regexp = '/#' . preg_quote($container_id, '/') . '\s+body/i';
$source = preg_replace($regexp, "#{$container_id}", $source);
}

// put block contents back in
$source = $replacements->resolve($source);

Expand Down
2 changes: 1 addition & 1 deletion skins/elastic/styles/styles.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ body.task-error-login #layout {
margin-bottom: .5rem;
}

div.rcmBody {
& > .rcmBody {
// Remove margins that can be set by the mail message styles
margin: 0 auto !important;
// Fix floating table issue (#9804)
Expand Down
10 changes: 10 additions & 0 deletions tests/ActionTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Roundcube\Tests;

use Masterminds\HTML5;
use PHPUnit\Framework\TestCase;

/**
Expand Down Expand Up @@ -268,6 +269,15 @@ protected static function loadSQLScript($db, $name)
}
}

/**
* Parse HTML output into DOMXPath object
*/
protected static function parseHtml($html)
{
$html5 = new HTML5(['disable_html_ns' => true]);
return new \DOMXPath($html5->loadHTML($html));
}

/**
* Call the action's run() method and handle exit exception
*/
Expand Down
38 changes: 38 additions & 0 deletions tests/Actions/Mail/ComposeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,44 @@ public function test_class()
$this->assertInstanceOf(\rcmail_action::class, $object);
}

/**
* Test prepare_html_body() method
*/
public function test_prepare_html_body()
{
$action = new \rcmail_action_mail_compose();

$html = <<<'EOF'
<html>
<head>
<style>
@media (min-width: 600px) {
.body_class_name { color: red; }
}
</style>
</head>
<body class="body_class_name" id="bod">
<p>Broken CSS selector</p>
</body>
</html>
EOF;

$body = $action->prepare_html_body($html);
$xpath = self::parseHtml($body);

$this->assertCount(1, $xpath->query('//style'));
$this->assertSame('text/css', $xpath->query('//style')->item(0)->getAttribute('type'));
$this->assertSame(
'@media (min-width: 600px) { .v1body_class_name { color: red; } }',
trim(preg_replace('/(\s{2,}|\n)/', ' ', $xpath->query('//style')->item(0)->textContent))
);

$this->assertCount(1, $xpath->query('//div'));
$this->assertSame('v1bod', $xpath->query('//div')->item(0)->getAttribute('id'));
$this->assertSame('rcmBody v1body_class_name', $xpath->query('//div')->item(0)->getAttribute('class'));
$this->assertCount(1, $xpath->query('//div/p'));
}

/**
* Test quote_text() method
*/
Expand Down
Loading
Loading