[Rt-commit] rt branch, 3.9-trunk, updated. rt-3.9.7-992-g438a9f3
Thomas Sibley
trs at bestpractical.com
Tue Dec 21 18:16:24 EST 2010
The branch, 3.9-trunk has been updated
via 438a9f30d49ff6f56abb013b228f9e5dc77c6989 (commit)
via 4024f896144612e44851a470ddcac3ccec0c9eca (commit)
via 9f386ab048dd55118a7d23475170fee34afde066 (commit)
via 1aff7c68dd88c7f6c2904c5c09e4c0e7a2bba066 (commit)
from 5651f1b83741816c4fecf6863080ebe036a618f8 (commit)
Summary of changes:
lib/RT/Interface/Web.pm | 66 +++++++++++++++++++++++++++++++++++
sbin/rt-test-dependencies.in | 1 +
share/html/Admin/Global/Theme.html | 4 ++
share/html/Admin/Tools/Queries.html | 2 +-
share/html/Elements/ScrubHTML | 27 +--------------
t/web/scrub.t | 46 ++++++++++++++++++++++++
6 files changed, 119 insertions(+), 27 deletions(-)
create mode 100644 t/web/scrub.t
- Log -----------------------------------------------------------------
commit 1aff7c68dd88c7f6c2904c5c09e4c0e7a2bba066
Author: Thomas Sibley <trs at bestpractical.com>
Date: Tue Dec 21 12:18:40 2010 -0500
Add a missing period
diff --git a/share/html/Admin/Tools/Queries.html b/share/html/Admin/Tools/Queries.html
index 4d16fd3..a01bbaf 100644
--- a/share/html/Admin/Tools/Queries.html
+++ b/share/html/Admin/Tools/Queries.html
@@ -48,7 +48,7 @@
<%init>
my $title = loc('SQL Queries');
unless ($session{'CurrentUser'}->HasRight( Object=> $RT::System, Right => 'SuperUser')) {
- Abort(loc('This feature is only available to system administrators'));
+ Abort(loc('This feature is only available to system administrators.'));
}
</%init>
<& /Admin/Elements/Header, Title => $title &>
commit 9f386ab048dd55118a7d23475170fee34afde066
Author: Thomas Sibley <trs at bestpractical.com>
Date: Tue Dec 21 12:23:26 2010 -0500
Lock down the theme editor to SuperUsers only
diff --git a/share/html/Admin/Global/Theme.html b/share/html/Admin/Global/Theme.html
index 160be19..ce50d2c 100644
--- a/share/html/Admin/Global/Theme.html
+++ b/share/html/Admin/Global/Theme.html
@@ -183,6 +183,10 @@ jQuery(function($) {
});
</script>
<%INIT>
+unless ($session{'CurrentUser'}->HasRight( Object=> RT->System, Right => 'SuperUser')) {
+ Abort(loc('This feature is only available to system administrators.'));
+}
+
require JSON;
my $text_threshold = 0.6;
commit 4024f896144612e44851a470ddcac3ccec0c9eca
Author: Thomas Sibley <trs at bestpractical.com>
Date: Tue Dec 21 17:59:37 2010 -0500
Move the meat of ScrubHTML into RT::Interface::Web::ScrubHTML
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 6774779..694b40c 100755
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -2541,6 +2541,56 @@ sub _parse_saved_search {
return ( _load_container_object( $obj_type, $obj_id ), $search_id );
}
+=head2 ScrubHTML content
+
+Removes unsafe and undesired HTML from the passed content
+
+=cut
+
+my $SCRUBBER;
+sub ScrubHTML {
+ my $Content = shift;
+ $SCRUBBER = _NewScrubber() unless $SCRUBBER;
+
+ $Content = '' if !defined($Content);
+ return $SCRUBBER->scrub($Content);
+}
+
+=head2 _NewScrubber
+
+Returns a new L<HTML::Scrubber> object. Override this if you insist on
+letting more HTML through.
+
+=cut
+
+sub _NewScrubber {
+ require HTML::Scrubber;
+ my $scrubber = HTML::Scrubber->new();
+ $scrubber->default(
+ 0,
+ {
+ '*' => 0,
+ id => 1,
+ class => 1,
+ # Match http, ftp and relative urls
+ # XXX: we also scrub format strings with this module then allow simple config options
+ href => qr{^(?:http:|ftp:|https:|/|__Web(?:Path|BaseURL|URL)__)}i,
+ face => 1,
+ size => 1,
+ target => 1,
+ style => qr{^(?:(?:color:\s*rgb\(\d+,\s*\d+,\s*\d+\))|
+ (?:text-align:\s*))}ix,
+ }
+ );
+ $scrubber->deny(qw[*]);
+ $scrubber->allow(
+ qw[A B U P BR I HR BR SMALL EM FONT SPAN STRONG SUB SUP STRIKE H1 H2 H3 H4 H5 H6 DIV UL OL LI DL DT DD PRE]
+ );
+ $scrubber->comment(0);
+
+ return $scrubber;
+}
+
RT::Base->_ImportOverlays();
1;
diff --git a/share/html/Elements/ScrubHTML b/share/html/Elements/ScrubHTML
index 662309e..47d4554 100644
--- a/share/html/Elements/ScrubHTML
+++ b/share/html/Elements/ScrubHTML
@@ -45,33 +45,8 @@
%# those contributions and any derivatives thereof.
%#
%# END BPS TAGGED BLOCK }}}
-<%ONCE>
-my $scrubber = HTML::Scrubber->new();
-$scrubber->default(
- 0,
- {
- '*' => 0,
- id => 1,
- class => 1,
- # Match http, ftp and relative urls
- # XXX: we also scrub format strings with this module then allow simple config options
- href => qr{^(?:http:|ftp:|https:|/|__Web(?:Path|BaseURL|URL)__)}i,
- face => 1,
- size => 1,
- target => 1,
- style => qr{^(?:(?:color:\s*rgb\(\d+,\s*\d+,\s*\d+\))|
- (?:text-align:\s*))}ix,
- }
-);
-$scrubber->deny(qw[*]);
-$scrubber->allow(
- qw[A B U P BR I HR BR SMALL EM FONT SPAN STRONG SUB SUP STRIKE H1 H2 H3 H4 H5 H6 DIV UL OL LI DL DT DD PRE]
-);
-$scrubber->comment(0);
-</%ONCE>
<%init>
-$Content = '' if !defined($Content);
-return $scrubber->scrub($Content);
+return ScrubHTML($Content);
</%init>
<%args>
$Content => undef
commit 438a9f30d49ff6f56abb013b228f9e5dc77c6989
Author: Thomas Sibley <trs at bestpractical.com>
Date: Tue Dec 21 18:05:46 2010 -0500
Overhaul what CSS we allow in style attributes to be safer *and* more useful
The regex takes care to accomodate Outlook's idiosyncrasies and ensure
that the features of the rich text editor work.
This is still a large hammer. If there's any forbidden CSS property
specified the entire style attribute will be stripped (including
otherwise allowed properties). The fix for this is parsing the CSS
itself with something like CSS::Adaptor::Whitelist, but that's for
another day.
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 694b40c..7292780 100755
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -2578,8 +2578,24 @@ sub _NewScrubber {
face => 1,
size => 1,
target => 1,
- style => qr{^(?:(?:color:\s*rgb\(\d+,\s*\d+,\s*\d+\))|
- (?:text-align:\s*))}ix,
+ style => qr{
+ ^(?:\s*
+ (?:(?:background-)?color: \s*
+ (?:rgb\(\s* \d+, \s* \d+, \s* \d+ \s*\) | # rgb(d,d,d)
+ \#[a-f0-9]{3,6} | # #fff or #ffffff
+ [\w\-]+ # green, light-blue, etc.
+ ) |
+ text-align: \s* \w+ |
+ font-size: \s* [\w.\-]+ |
+ font-family: \s* [\w\s"',.\-]+ |
+ font-weight: \s* [\w\-]+ |
+
+ # MS Office styles, which are probably fine. If we don't, then any
+ # associated styles in the same attribute get stripped.
+ mso-[\w\-]+?: \s* [\w\s"',.\-]+
+ )\s* ;? \s*)
+ +$ # one or more of these allowed properties from here 'till sunset
+ }ix,
}
);
$scrubber->deny(qw[*]);
diff --git a/sbin/rt-test-dependencies.in b/sbin/rt-test-dependencies.in
index 506ecd3..3b3b1c7 100755
--- a/sbin/rt-test-dependencies.in
+++ b/sbin/rt-test-dependencies.in
@@ -279,6 +279,7 @@ Test::MockTime
Log::Dispatch::Perl
Test::WWW::Mechanize::PSGI
Plack::Middleware::Test::StashWarnings
+Test::LongString
.
$deps{'FASTCGI'} = [ text_to_hash( << '.') ];
diff --git a/t/web/scrub.t b/t/web/scrub.t
new file mode 100644
index 0000000..6483a75
--- /dev/null
+++ b/t/web/scrub.t
@@ -0,0 +1,46 @@
+#!/usr/bin/perl
+use strict;
+use warnings;
+
+use RT::Test nodb => 1, tests => 6;
+use RT::Interface::Web; # This gets us HTML::Mason::Commands
+use Test::LongString;
+
+{
+ my $html = 'This is a test of <span style="color: rgb(255, 0, 0); ">color</span> and <span style="font-size: 18px; "><span style="font-family: Georgia, serif; ">font</span></span> and <em><u><strike><strong>boldness</strong></strike></u></em>.';
+ is_string(scrub_html($html), $html, "CKEditor produced HTML sails through");
+}
+
+{
+ my $html = '<p style="text-align: right; ">
+ And <span style="color: rgb(255, 0, 0); "><span style="font-size: 16px; "><span style="font-family: Georgia, serif; ">alignment with color</span></span></span>?</p>';
+ is_string(scrub_html($html), $html, "CKEditor produced HTML sails through");
+}
+
+{
+ my $html = 'This is a test of <span style="color: rgb(255, 0, 0); content: url(/Nasty/URL);">color</span> and <span style="font-size: 18px; "><span style="font-family: Georgia, serif; ">font</span></span> and <em><u><strike><strong>boldness</strong></strike></u></em>.';
+ my $expected = 'This is a test of <span>color</span> and <span style="font-size: 18px; "><span style="font-family: Georgia, serif; ">font</span></span> and <em><u><strike><strong>boldness</strong></strike></u></em>.';
+ is_string(scrub_html($html), $expected, "nasty CSS not allowed through");
+}
+
+{
+ my $html = 'Let\'s add some <span style="color: blue; font-family: Georgia">color</span> up in <span style="color: #DEADBE">here</span>.';
+ is_string(scrub_html($html), $html, "multiple props and color specs allowed");
+}
+
+{
+ my $html = q[<span lang=EN-US style='font-family:"Century Gothic","sans-serif";'>oh hai I'm some text</span>];
+ my $expected = q[<span style="font-family:"Century Gothic","sans-serif";">oh hai I'm some text</span>];
+ is_string(scrub_html($html), $expected, "font lists");
+}
+
+{
+ my $html = q[<span lang=EN-US style='font-size:7.5pt;font-family:"Century Gothic","sans-serif";color:#666666;mso-fareast-language:IT'>oh hai I'm some text</span>];
+ my $expected = q[<span style="font-size:7.5pt;font-family:"Century Gothic","sans-serif";color:#666666;mso-fareast-language:IT">oh hai I'm some text</span>];
+ is_string(scrub_html($html), $expected, "outlook html");
+}
+
+sub scrub_html {
+ return HTML::Mason::Commands::ScrubHTML(shift);
+}
+
-----------------------------------------------------------------------
More information about the Rt-commit
mailing list