[Rt-commit] rt branch, 4.2/html-external-formatter, created. rt-4.2.9-99-g6337505
Alex Vandiver
alexmv at bestpractical.com
Tue Feb 3 13:25:11 EST 2015
The branch, 4.2/html-external-formatter has been created
at 6337505bc81b49b8d402a8dda12a3ddef56eb411 (commit)
- Log -----------------------------------------------------------------
commit c40ba502ce56fe1d948d0593d9172989d96a80b0
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Thu Jan 2 21:37:16 2014 -0500
Use HTML::FormatExternal to allow external HTML → text formatters
The HTML::FormatText::WithLinks::AndTables module is unmaintained, and
contains multiple bugs which trigger runtime errors. In such cases, RT
sends out an empty text/plain part -- if HTML templates are not in use,
this results in an entirely empty email.
Add HTML::FormatExternal as an optional dependency for more reliable
HTML to text conversion using external text-based web browsers and text
conversion tools.
Benchmarking shows that shelling out to an external program on complex
HTML is comparable in speed (or faster!) than the pure-perl formatter
(20/s). On simple HTML, the pure-perl formatter is an order of
magnitude faster -- but the slowest external formatter (elinks) still
runs at 5/sec, and most run at 20-50/s. This is judged to not be likely
to cause a performance impact, as it is run at most once per scrip.
diff --git a/docs/UPGRADING-4.2 b/docs/UPGRADING-4.2
index 2ed98e4..3ab005e 100644
--- a/docs/UPGRADING-4.2
+++ b/docs/UPGRADING-4.2
@@ -345,4 +345,12 @@ have been overridden by CSS since 4.0.0, and thus did not affect
display. They have been removed, and setting them will trigger an
informational message that setting them is ineffective.
+=head1 UPGRADING FROM 4.2.9 AND EARLIER
+
+An additional optional dependency, L<HTML::FormatExternal>, has been
+added. This allows RT to use C<w3m>, C<elinks>, C<html2text>, or other
+external tools to render HTML to text. This dependency is not installed
+by default; however, its use is strongly encouraged, and will resolve
+issues with blank outgoing emails.
+
=cut
diff --git a/etc/RT_Config.pm.in b/etc/RT_Config.pm.in
index 294b255..591feec 100644
--- a/etc/RT_Config.pm.in
+++ b/etc/RT_Config.pm.in
@@ -703,6 +703,26 @@ will use the address of the current user and remove RT's subject tag.
Set($ForwardFromUser, 0);
+=item C<$HTMLFormatter>
+
+RT's default pure-perl formatter may fail to successfully convert even
+on some relatively simple HTML; this will result in blank C<text/plain>
+parts, which is particuarly unfortunate if HTML templates are not in
+use.
+
+If the optional dependency L<HTML::FormatExternal> is installed, RT will
+use external programs to render HTML to plain text. The default is to
+try, in order, C<w3m>, C<elinks>, C<html2text>, C<links>, C<lynx>, and
+then fall back to the C<core> pure-perl formatter if none are installed.
+
+Set C<$HTMLFormatter> to one of the above programs (or the full path to
+such) to use a different program than the above would choose by default.
+Setting this requires that L<HTML::FormatExternal> be installed.
+
+=cut
+
+Set($HTMLFormatter, undef);
+
=back
=head2 Email dashboards
diff --git a/lib/RT/Config.pm b/lib/RT/Config.pm
index 9a1bef8..b6bfce5 100644
--- a/lib/RT/Config.pm
+++ b/lib/RT/Config.pm
@@ -631,6 +631,10 @@ our %META;
$self->Set( MailCommand => 'sendmailpipe' );
},
},
+ HTMLFormatter => {
+ Type => 'SCALAR',
+ PostLoadCheck => sub { RT::Interface::Email->_HTMLFormatter },
+ },
MailPlugins => {
Type => 'ARRAY',
PostLoadCheck => sub {
diff --git a/lib/RT/Interface/Email.pm b/lib/RT/Interface/Email.pm
index cd6b332..8830070 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -50,6 +50,7 @@ package RT::Interface::Email;
use strict;
use warnings;
+use 5.010;
use Email::Address;
use MIME::Entity;
@@ -57,6 +58,7 @@ use RT::EmailParser;
use File::Temp;
use Mail::Mailer ();
use Text::ParseWords qw/shellwords/;
+use RT::Util 'safe_run_child';
BEGIN {
use base 'Exporter';
@@ -1794,9 +1796,64 @@ conversion fails.
=cut
sub ConvertHTMLToText {
+ return _HTMLFormatter()->(@_);
+}
+
+sub _HTMLFormatter {
+ state $formatter;
+ return $formatter if defined $formatter;
+
+ my $wanted = RT->Config->Get("HTMLFormatter");
+
+ my @order;
+ if ($wanted) {
+ @order = ($wanted, "core");
+ } else {
+ @order = ("w3m", "elinks", "html2text", "links", "lynx", "core");
+ }
+ # Always fall back to core, even if it is not listed
+ for my $prog (@order) {
+ if ($prog eq "core") {
+ RT->Logger->info("Using internal Perl HTML -> text conversion");
+ require HTML::FormatText::WithLinks::AndTables;
+ $formatter = \&_HTMLFormatText;
+ } else {
+ unless (HTML::FormatExternal->require) {
+ RT->Logger->warn("HTML::FormatExternal is not installed; falling back to internal perl formatter")
+ if $wanted;
+ next;
+ }
+
+ my $package = "HTML::FormatText::" . ucfirst($prog);
+ unless ($package->require) {
+ RT->Logger->warn("$prog is not a valid formatter provided by HTML::FormatExternal")
+ if $wanted;
+ next;
+ }
+
+ unless (defined $package->program_version) {
+ RT->Logger->warn("Could not find external '$prog' HTML formatter; is it installed?")
+ if $wanted;
+ next;
+ }
+
+ RT->Logger->info("Using $prog for HTML -> text conversion");
+ $formatter = sub {
+ my $html = shift;
+ RT::Util::safe_run_child {
+ $package->format_string($html, leftmargin => 0, rightmargin => 78);
+ };
+ };
+ }
+ RT->Config->Set( HTMLFormatter => $prog );
+ last;
+ }
+ return $formatter;
+}
+
+sub _HTMLFormatText {
my $html = shift;
- require HTML::FormatText::WithLinks::AndTables;
my $text;
eval {
$text = HTML::FormatText::WithLinks::AndTables->convert(
commit 23b64b50fb69c12d124e905ffafe5398bf8b3919
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Wed Jun 18 18:44:52 2014 -0400
Allow $HTMLFormatter to contain the full path
diff --git a/etc/RT_Config.pm.in b/etc/RT_Config.pm.in
index 591feec..c3f7890 100644
--- a/etc/RT_Config.pm.in
+++ b/etc/RT_Config.pm.in
@@ -719,6 +719,9 @@ Set C<$HTMLFormatter> to one of the above programs (or the full path to
such) to use a different program than the above would choose by default.
Setting this requires that L<HTML::FormatExternal> be installed.
+If the chosen formatter is not in the webserver's $PATH, you may set
+this option the full path to one of the aforementioned executables.
+
=cut
Set($HTMLFormatter, undef);
diff --git a/lib/RT/Interface/Email.pm b/lib/RT/Interface/Email.pm
index 8830070..b89d467 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -1824,6 +1824,7 @@ sub _HTMLFormatter {
next;
}
+ my $path = $prog =~ s{(.*/)}{} ? $1 : undef;
my $package = "HTML::FormatText::" . ucfirst($prog);
unless ($package->require) {
RT->Logger->warn("$prog is not a valid formatter provided by HTML::FormatExternal")
@@ -1831,8 +1832,15 @@ sub _HTMLFormatter {
next;
}
- unless (defined $package->program_version) {
- RT->Logger->warn("Could not find external '$prog' HTML formatter; is it installed?")
+ if ($path) {
+ local $ENV{PATH} = $path;
+ if (not defined $package->program_version) {
+ RT->Logger->warn("Could not find or run external '$prog' HTML formatter in $path$prog")
+ if $wanted;
+ next;
+ }
+ } elsif (not defined $package->program_version) {
+ RT->Logger->warn("Could not find or run external '$prog' HTML formatter in \$PATH ($ENV{PATH}) -- you may need to install it or provide the full path")
if $wanted;
next;
}
@@ -1841,6 +1849,7 @@ sub _HTMLFormatter {
$formatter = sub {
my $html = shift;
RT::Util::safe_run_child {
+ local $ENV{PATH} = $path || $ENV{PATH};
$package->format_string($html, leftmargin => 0, rightmargin => 78);
};
};
commit 24365b3f3cbeaf6d8f9a31fd59dbd8628e0bafb8
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Wed Jun 25 21:38:19 2014 -0400
Provide a default PATH in cases where it is unset (mod_fastcgi)
See 4ef0d833 for the full explanation, which applies in this context as
well.
diff --git a/lib/RT/Interface/Email.pm b/lib/RT/Interface/Email.pm
index b89d467..39fdabc 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -1839,17 +1839,22 @@ sub _HTMLFormatter {
if $wanted;
next;
}
- } elsif (not defined $package->program_version) {
- RT->Logger->warn("Could not find or run external '$prog' HTML formatter in \$PATH ($ENV{PATH}) -- you may need to install it or provide the full path")
- if $wanted;
- next;
+ } else {
+ local $ENV{PATH} = '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin'
+ unless defined $ENV{PATH};
+ if (not defined $package->program_version) {
+ RT->Logger->warn("Could not find or run external '$prog' HTML formatter in \$PATH ($ENV{PATH}) -- you may need to install it or provide the full path")
+ if $wanted;
+ next;
+ }
}
RT->Logger->info("Using $prog for HTML -> text conversion");
$formatter = sub {
my $html = shift;
RT::Util::safe_run_child {
- local $ENV{PATH} = $path || $ENV{PATH};
+ local $ENV{PATH} = $path || $ENV{PATH}
+ || '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin';
$package->format_string($html, leftmargin => 0, rightmargin => 78);
};
};
commit 927ece5881ec232db1dbaa53fcb32cc7b1d036bf
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Wed Dec 31 14:42:17 2014 -0500
Set an $ENV{HOME} that the external formatters can write under
Many of them attempt to read a configuration file or directory under
$ENV{HOME}; some (like w3m) even attempt to write a blank template of
one if it does not exist.
Provide a suitable $ENV{HOME} which will be writable. This prevents
errors being written to STDERR (and thus the webserver error log) on
every invocation.
diff --git a/lib/RT/Interface/Email.pm b/lib/RT/Interface/Email.pm
index 39fdabc..b1ac2bd 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -1834,6 +1834,7 @@ sub _HTMLFormatter {
if ($path) {
local $ENV{PATH} = $path;
+ local $ENV{HOME} = $RT::VarPath;
if (not defined $package->program_version) {
RT->Logger->warn("Could not find or run external '$prog' HTML formatter in $path$prog")
if $wanted;
@@ -1842,6 +1843,7 @@ sub _HTMLFormatter {
} else {
local $ENV{PATH} = '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin'
unless defined $ENV{PATH};
+ local $ENV{HOME} = $RT::VarPath;
if (not defined $package->program_version) {
RT->Logger->warn("Could not find or run external '$prog' HTML formatter in \$PATH ($ENV{PATH}) -- you may need to install it or provide the full path")
if $wanted;
@@ -1855,6 +1857,7 @@ sub _HTMLFormatter {
RT::Util::safe_run_child {
local $ENV{PATH} = $path || $ENV{PATH}
|| '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin';
+ local $ENV{HOME} = $RT::VarPath;
$package->format_string($html, leftmargin => 0, rightmargin => 78);
};
};
commit 6337505bc81b49b8d402a8dda12a3ddef56eb411
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Fri Jan 3 17:16:38 2014 -0500
Update tests to work with any of the HTML formatters
diff --git a/t/api/txn_content.t b/t/api/txn_content.t
index d44862d..672d6c2 100644
--- a/t/api/txn_content.t
+++ b/t/api/txn_content.t
@@ -19,5 +19,5 @@ ok( $txn, 'got Create txn' );
# ->Content converts from text/html to plain text if we don't explicitly ask
# for html. Our html -> text converter seems to add an extra trailing newline
-is( $txn->Content, "this is body\n\n", "txn's html content converted to plain text" );
+like( $txn->Content, qr/^\s*this is body\s*$/, "txn's html content converted to plain text" );
is( $txn->Content(Type => 'text/html'), "this is body\n", "txn's html content" );
diff --git a/t/mail/html-outgoing.t b/t/mail/html-outgoing.t
index 6a0f2be..a37f52c 100644
--- a/t/mail/html-outgoing.t
+++ b/t/mail/html-outgoing.t
@@ -39,7 +39,7 @@ mail_ok {
to => 'enduser at example.com',
subject => qr/\Q[example.com #1] AutoReply: The internet is broken\E/,
body => parts_regex(
- 'trouble ticket regarding The internet is broken',
+ 'trouble ticket regarding \*?The internet is broken\*?',
'trouble ticket regarding <b>The internet is broken</b>'
),
'Content-Type' => qr{multipart},
@@ -47,7 +47,7 @@ mail_ok {
bcc => 'root at localhost',
subject => qr/\Q[example.com #1] The internet is broken\E/,
body => parts_regex(
- 'Request 1 \(http://localhost:\d+/Ticket/Display\.html\?id=1\)\s+?was acted upon by RT_System',
+ 'Request (\[\d+\])?1(\s*[(<]http://localhost:\d+/Ticket/Display\.html\?id=1[)>])?\s*was acted upon by RT_System',
'Request <a href="http://localhost:\d+/Ticket/Display\.html\?id=1">1</a> was acted upon by RT_System\.</b>'
),
'Content-Type' => qr{multipart},
@@ -65,8 +65,8 @@ mail_ok {
bcc => 'root at localhost',
subject => qr/\Q[example.com #1] The internet is broken\E/,
body => parts_regex(
- 'Ticket URL: http://localhost:\d+/Ticket/Display\.html\?id=1.+?'.
- 'This is a test of HTML correspondence\.',
+ 'Ticket URL: (?:\[\d+\])?http://localhost:\d+/Ticket/Display\.html\?id=1.+?'.
+ 'This is a test of \*?HTML\*? correspondence\.',
'Ticket URL: <a href="(http://localhost:\d+/Ticket/Display\.html\?id=1)">\1</a>.+?'.
'<p>This is a test of <b>HTML</b> correspondence\.</p>'
),
@@ -75,35 +75,39 @@ mail_ok {
to => 'enduser at example.com',
subject => qr/\Q[example.com #1] The internet is broken\E/,
body => parts_regex(
- 'This is a test of HTML correspondence\.',
+ 'This is a test of \*?HTML\*? correspondence\.',
'<p>This is a test of <b>HTML</b> correspondence\.</p>'
),
'Content-Type' => qr{multipart},
};
+SKIP: {
+ skip "Only fails on core HTMLFormatter", 9
+ unless RT->Config->Get("HTMLFormatter") eq "core";
+ diag "Failing HTML -> Text conversion";
+ warnings_like {
+ my $body = '<table><tr><td><table><tr><td>Foo</td></tr></table></td></tr></table>';
+ mail_ok {
+ ($ok, $tmsg) = $t->Correspond(
+ MIMEObj => HTML::Mason::Commands::MakeMIMEEntity(
+ Body => $body,
+ Type => 'text/html',
+ ),
+ );
+ } { from => qr/RT System/,
+ bcc => 'root at localhost',
+ subject => qr/\Q[example.com #1] The internet is broken\E/,
+ body => qr{Ticket URL: <a href="(http://localhost:\d+/Ticket/Display\.html\?id=1)">\1</a>.+?$body}s,
+ 'Content-Type' => qr{text/html}, # TODO
+ },{ from => qr/RT System/,
+ to => 'enduser at example.com',
+ subject => qr/\Q[example.com #1] The internet is broken\E/,
+ body => qr{$body},
+ 'Content-Type' => qr{text/html}, # TODO
+ };
+ } [(qr/uninitialized value/, qr/Failed to downgrade HTML/)x3];
+}
-diag "Failing HTML -> Text conversion";
-warnings_like {
- my $body = '<table><tr><td><table><tr><td>Foo</td></tr></table></td></tr></table>';
- mail_ok {
- ($ok, $tmsg) = $t->Correspond(
- MIMEObj => HTML::Mason::Commands::MakeMIMEEntity(
- Body => $body,
- Type => 'text/html',
- ),
- );
- } { from => qr/RT System/,
- bcc => 'root at localhost',
- subject => qr/\Q[example.com #1] The internet is broken\E/,
- body => qr{Ticket URL: <a href="(http://localhost:\d+/Ticket/Display\.html\?id=1)">\1</a>.+?$body}s,
- 'Content-Type' => qr{text/html}, # TODO
- },{ from => qr/RT System/,
- to => 'enduser at example.com',
- subject => qr/\Q[example.com #1] The internet is broken\E/,
- body => qr{<table><tr><td><table><tr><td>Foo</td></tr></table></td></tr></table>},
- 'Content-Type' => qr{text/html}, # TODO
- };
-} [(qr/uninitialized value/, qr/Failed to downgrade HTML/)x3];
diag "Admin Comment in HTML";
mail_ok {
@@ -117,9 +121,9 @@ mail_ok {
bcc => 'root at localhost',
subject => qr/\Q[example.com #1] [Comment] The internet is broken\E/,
body => parts_regex(
- 'This is a comment about ticket 1 \(http://localhost:\d+/Ticket/Display\.html\?id=1\)\..+?'.
+ 'This is a comment about (\[\d+\])?ticket.1(\s*[(<]http://localhost:\d+/Ticket/Display\.html\?id=1[)>])?\..+?'.
'It is not sent to the Requestor\(s\):.+?'.
- 'Comment test, please!',
+ 'Comment test, _?please!_?',
'<p>This is a comment about <a href="http://localhost:\d+/Ticket/Display\.html\?id=1">ticket 1</a>\. '.
'It is not sent to the Requestor\(s\):</p>.+?'.
-----------------------------------------------------------------------
More information about the rt-commit
mailing list