[Rt-commit] rt branch, 4.2/html-external-formatter, created. rt-4.2.9-65-g568f22a

Alex Vandiver alexmv at bestpractical.com
Wed Dec 31 14:43:30 EST 2014


The branch, 4.2/html-external-formatter has been created
        at  568f22a5db4aca745d68095fe6fab59c9fb8aee2 (commit)

- Log -----------------------------------------------------------------
commit d9507efe23b1a5ea4a0c9d62bdb1817f106cd23d
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.  Add
    HTML::FormatExternal as an optional dependency for slower but more
    reliable HTML to text conversion using external text-based web browsers
    and text conversion tools.

diff --git a/etc/RT_Config.pm.in b/etc/RT_Config.pm.in
index 294b255..6dd9192 100644
--- a/etc/RT_Config.pm.in
+++ b/etc/RT_Config.pm.in
@@ -703,6 +703,23 @@ will use the address of the current user and remove RT's subject tag.
 
 Set($ForwardFromUser, 0);
 
+=item C<$HTMLFormatter>
+
+The external HTML formatter to use.  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.  The pure-perl formatter is
+significantly faster than the others, but may fail to successfully
+convert even on some relatively simple HTML.  Falls back to C<core> if
+the configured program is not installed.
+
+Using formatters other than C<core> requires that the
+L<HTML::FormatExternal> module be installed; it is an optional
+dependency which RT will not install by default.
+
+=cut
+
+Set($HTMLFormatter, undef);
+
 =back
 
 =head2 Email dashboards
diff --git a/lib/RT/Config.pm b/lib/RT/Config.pm
index 4329fdb..e5e048c 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 d2e9bb5..32a300c 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 e21f24801c02dffa85438325b350989834e6a203
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 6dd9192..bb63e5d 100644
--- a/etc/RT_Config.pm.in
+++ b/etc/RT_Config.pm.in
@@ -716,6 +716,9 @@ Using formatters other than C<core> requires that the
 L<HTML::FormatExternal> module be installed; it is an optional
 dependency which RT will not install by default.
 
+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 32a300c..c203743 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 1a39cd198f1902e974f15e189756349155b2ce23
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 c203743..d82b601 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 296d5c2b42efa55335e6ff3848fe8839849f557f
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 d82b601..0dafe3d 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 568f22a5db4aca745d68095fe6fab59c9fb8aee2
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