998323 - URLs pasted in comments are no longer displayed

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Christian Ruppert]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release)
Build ID: 20140321194345

Steps to reproduce:

File a bug and post some URIs, e.g. http://www.google.com


Actual results:

The URIs will be replaced in Bugzilla/Template.pm by \0\0<ID>\0\0.
The control chars are stripped because of the change in Bugzilla/Util.pm:
$var =~ s/(?![\t\r\n])[[:cntrl:]]//g;

https://git.mozilla.org/?p=bugzilla/bugzilla.git;a=commitdiff;h=58b92d3b0245f6565a7ff34e78fce1e9ec56b355 [github]

The final URI will be just the <ID> instead of the quoted URI (<a href ...>)

XML, printing and XMLRPC etc. are not affected it seems.


Expected results:

URIs should be quoted properly.

Comment 1

[Christian Ruppert]

Attachment: 0001-html_quote-Don-t-purge-0-s-because-of-quoteUrls.patch

A proposed patch. Should do the trick.

Comment 2

[Christian Ruppert]

Hm, dunno but it might be a "major" issue as it will break some comments, at least when hitting the "reply" link. The quoted text will contain and keep the ID instead of the actual URI.

Comment 3

[Frédéric Buclin]

Confirmed. All branches are affected due to bug 968576.

Comment 4

[David Lawrence [:dkl]]

Attachment: 998323_1.patch

This fixes the problem without modifying html_quote() by using a random string for token replacements instead of the \0 control characters.

dkl

Comment 5

[Frédéric Buclin]

Comment on attachment 8409068 [details] [diff] [review]
998323_1.patch

>=== modified file 'Bugzilla/Template.pm'

>+    my $token = generate_random_password();

I think that instead of a token (which calls our cryptographic code every time we call quoteUrls()), we should use one of the reserved characters that Unicode offers, as we did to fix bug 405011, see bug 405011 comment 58. As we already use \x{FDD0} and \x{FDD1}, we could use e.g. \x{FDD2}.

Comment 6

[David Lawrence [:dkl]]

Attachment: 998323_2.patch

User \x{FDD2}$count\x{FDD3} instead if \0\0$count\0\0

dkl

Comment 7

[David Lawrence [:dkl]]

Attachment: Patch for 4.2, 4.4, trunk

Comment 8

[David Lawrence [:dkl]]

Comment on attachment 8409099 [details] [diff] [review]
998323_2.patch

Need to change comment from discussion in IRC

Comment 9

[David Lawrence [:dkl]]

Attachment: Patch for 4.0

Comment 10

[Frédéric Buclin]

Comment on attachment 8409099 [details] [diff] [review]
998323_2.patch

>+    # \x{FDDx} is used because it's unlikely to occur in the text,

I would say: # \x{FDDx} is a reserved character in Unicode and so is safe to use.

Or you could even drop the comment entirely. :)


>+    no warnings 'utf8';

Add a comment explaining that this is required for Perl 5.13.8 and older.


Otherwise looks good and works fine. r=LpSolit

Comment 11

[Frédéric Buclin]

Comment on attachment 8409114 [details] [diff] [review]
Patch for 4.0

>+    # and are reserved unicode characters. We disable warnings for now
>+    # until we require Perl 5.13.8 or newer.

5.13.8 is still affected. 5.13.9 is the first version to no longer complain. r=LpSolit with this fix on checkin.

Comment 12

[Frédéric Buclin]

Comment on attachment 8408965 [details] [diff] [review]
0001-html_quote-Don-t-purge-0-s-because-of-quoteUrls.patch

\0 is evil and must be removed.

Comment 13

[Frédéric Buclin]

Comment on attachment 8409112 [details] [diff] [review]
Patch for 4.2, 4.4, trunk

>+    # and are reserved unicode characters. We disable warnings for now
>+    # until we require Perl 5.13.8 or newer.

5.13.9 or newer. r=LpSolit

Comment 14

[David Lawrence [:dkl]]

To ssh://[e-mail]/bugzilla/bugzilla.git
   406dcbb..f409dc5  master -> master

To ssh://[e-mail]/bugzilla/bugzilla.git
   3f79ec4..af76027  4.4 -> 4.4

To ssh://[e-mail]/bugzilla/bugzilla.git
   b909fa7..12f4d73  4.2 -> 4.2

To ssh://[e-mail]/bugzilla/bugzilla.git
   aa71c23..1f44ef2  4.0 -> 4.0