[Rt-commit] rt branch, 4.2-trunk, updated. rt-4.2.13-127-g0d278b5
Shawn Moore
shawn at bestpractical.com
Mon May 22 16:40:57 EDT 2017
The branch, 4.2-trunk has been updated
via 0d278b51e86f59d12055fe1154b2ba1304371529 (commit)
via 53ad481ac0f38937bacd2699b48ac386a263ca03 (commit)
via a440e65a8faf25f051f7b2d3a17042b722ce2224 (commit)
from ddda65b0ec7f0fd7663219af5ba4a9751c352aeb (commit)
Summary of changes:
share/html/Elements/MakeClicky | 6 ++--
t/data/plugins/MakeClicky/html/makeclicky | 3 +-
t/web/make-clicky.t | 51 ++++++++++++++++++++++++++++---
3 files changed, 51 insertions(+), 9 deletions(-)
- Log -----------------------------------------------------------------
commit a440e65a8faf25f051f7b2d3a17042b722ce2224
Author: Alex Vandiver <alex at chmrr.net>
Date: Mon Feb 20 16:13:11 2017 -0800
Stop double-escaping HTML which is made into links
The MakeClicky component does two things: escapes plain-text content,
and makes inline references to URLs into <a href="..."> links.
It does this by using a regex to find all URL-like things; content
between them is left as-is (if it was originally HTML) or is
HTML-escaped (if it was plaintext).
The replacement, however, is _always_ run on matched content before it
is output. The result is that a link in HTML with an ampersand, found
as the characters "http://foo/?bar=baz&troz=zort" is HTML-escaped
once again, in both the link and the output, to create:
<a href="http://foo/?bar=baz&troz=zort">http://foo/?bar=baz&troz=zort</a>
...which is entirely incorrect.
Skip the additional HTML-escaping if the content passed in is already
HTML. It is not MakeClicky's domain to ensure that HTML is
well-formed and safe -- that is performed by /Elements/ScrubHTML. As
such, there is no harm in passing through matched content as-is.
Fixes: I#31169
diff --git a/share/html/Elements/MakeClicky b/share/html/Elements/MakeClicky
index c991c7b..252711b 100644
--- a/share/html/Elements/MakeClicky
+++ b/share/html/Elements/MakeClicky
@@ -63,7 +63,7 @@ my %actions = (
my %args = @_;
my $post = "";
$post = ")" if $args{value} !~ /\(/ and $args{value} =~ s/\)$//;
- $args{value} = $escaper->($args{value});
+ $args{value} = $escaper->($args{value}) unless $args{html};
my $result = qq{[<a target="_blank" href="$args{value}">}. loc('Open URL') .qq{</a>]};
return $args{value} . qq{ <span class="clickylink">$result</span>$post};
},
@@ -71,7 +71,7 @@ my %actions = (
my %args = @_;
my $post = "";
$post = ")" if $args{value} !~ /\(/ and $args{value} =~ s/\)$//;
- $args{value} = $escaper->($args{value});
+ $args{value} = $escaper->($args{value}) unless $args{html};
my $result = qq{<a target="_blank" href="$args{value}">$args{value}</a>};
return qq{<span class="clickylink">$result</span>$post};
},
@@ -169,7 +169,7 @@ while ( $$content =~ /($regexp)/gsio ) {
$pos += length($plain);
}
my $plain = $handle->(
- %ARGS,
+ %ARGS,
value => $match,
all_matches => [ $1, $2, $3, $4, $5, $6, $7, $8, $9 ],
);
diff --git a/t/data/plugins/MakeClicky/html/makeclicky b/t/data/plugins/MakeClicky/html/makeclicky
index aef660f..c329880 100644
--- a/t/data/plugins/MakeClicky/html/makeclicky
+++ b/t/data/plugins/MakeClicky/html/makeclicky
@@ -1,8 +1,9 @@
<%args>
$content => ""
+$html => 0
</%args>
<%init>
-$m->comp("/Elements/MakeClicky", content => \$content);
+$m->comp("/Elements/MakeClicky", content => \$content, html => $html);
$m->out($content);
$m->abort;
</%init>
diff --git a/t/web/make-clicky.t b/t/web/make-clicky.t
index a5efe86..da99d11 100644
--- a/t/web/make-clicky.t
+++ b/t/web/make-clicky.t
@@ -26,21 +26,55 @@ diag "Trailing punctuation";
diag "Punctuation as part of the url";
{
- my $url = 'http://bestpractical.com/rt/download.html?foo=bar,baz&bat=1.2';
- my $escaped_url = $url;
- RT::Interface::Web::EscapeHTML( \$escaped_url );
+ my $url = 'http://bestpractical.com/rt/download.html?foo=bar,baz.2';
is_string(
make_clicky($m, "Refer to $url. A following sentence."),
- qq[Refer to <span class="clickylink"><a target="_blank" href="$escaped_url">$escaped_url</a></span>. A following sentence.],
+ qq[Refer to <span class="clickylink"><a target="_blank" href="$url">$url</a></span>. A following sentence.],
"Punctuation in middle of URL",
);
}
+diag "Anchor in URL";
+{
+ my $url = 'http://wiki.bestpractical.com/test#anchor';
+ is_string(
+ make_clicky($m, "Anchor $url here"),
+ qq[Anchor <span class="clickylink"><a target="_blank" href="$url">$url</a></span> here],
+ "Captured anchor in URL",
+ );
+}
+
+diag "Query parameters in URL";
+for my $html (0, 1) {
+ my $url = "https://wiki.bestpractical.com/?q=test&search=1";
+ my $escaped_url = $url;
+ RT::Interface::Web::EscapeHTML( \$escaped_url );
+ is_string(
+ make_clicky($m, $html ? $escaped_url : $url, $html),
+ qq[<span class="clickylink"><a target="_blank" href="$escaped_url">$escaped_url</a></span>],
+ "Single escaped @{[$html ? 'HTML' : 'text']} query parameters",
+ );
+}
+
+diag "Found in href";
+{
+ my $url = '<a href="http://bestpractical.com/rt">Best Practical</a>';
+ is_string( make_clicky($m, $url, 1), $url, "URL in existing href is a no-op" );
+}
+
+diag "Found in other attribute";
+{
+ my $url = '<img src="http://bestpractical.com/rt" alt="Some image" />';
+ is_string( make_clicky($m, $url, 1), $url, "URL in image src= is a no-op" );
+}
+
+
sub make_clicky {
my $m = shift;
my $text = shift;
+ my $html = shift || 0;
RT::Interface::Web::EscapeURI(\$text);
- $m->get_ok("/makeclicky?content=$text", "made clicky")
+ $m->get_ok("/makeclicky?content=$text&html=$html", "made clicky")
or diag $m->status;
return $m->success ? $m->content : "";
}
commit 53ad481ac0f38937bacd2699b48ac386a263ca03
Author: Dave Goehrig <dave at bestpractical.com>
Date: Thu Mar 16 14:34:25 2017 -0400
Adding test to check double encoding &
Added a case where the url contains an &
if the html parameter is set to true, then it
should not reencode the & to #38amp;.
diff --git a/t/web/make-clicky.t b/t/web/make-clicky.t
index da99d11..43f4249 100644
--- a/t/web/make-clicky.t
+++ b/t/web/make-clicky.t
@@ -68,6 +68,13 @@ diag "Found in other attribute";
is_string( make_clicky($m, $url, 1), $url, "URL in image src= is a no-op" );
}
+diag "Do not double encode & test";
+{
+ my $url = 'http://bestpractical.com/search?q=me&irene;token=foo';
+ my $string = qq[<span class="clickylink"><a target="_blank" href="http://bestpractical.com/search?q=me&irene;token=foo">http://bestpractical.com/search?q=me&irene;token=foo</a></span>];
+ is_string( make_clicky($m,$url, 1), $string, "URL with & should not rencode" );
+
+}
sub make_clicky {
my $m = shift;
commit 0d278b51e86f59d12055fe1154b2ba1304371529
Merge: ddda65b 53ad481
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Mon May 22 20:40:36 2017 +0000
Merge branch '4.2/clicky-escaping' into 4.2-trunk
-----------------------------------------------------------------------
More information about the rt-commit
mailing list