URLs pasted in comments are no longer displayed

RESOLVED FIXED in Bugzilla 4.0

Status

()

Bugzilla
Creating/Changing Bugs
--
major
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Christian Ruppert, Assigned: dkl)

Tracking

({regression})

4.4.3
Bugzilla 4.0
regression
Dependency tree / graph
Bug Flags:
approval +
approval4.4 +
blocking4.4.4 +
approval4.2 +
blocking4.2.9 +
approval4.0 +
blocking4.0.13 +

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

3 years ago
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.
(Reporter)

Comment 1

3 years ago
Created attachment 8408965 [details] [diff] [review]
0001-html_quote-Don-t-purge-0-s-because-of-quoteUrls.patch

A proposed patch. Should do the trick.
(Reporter)

Updated

3 years ago
Version: unspecified → 4.4.3
(Reporter)

Comment 2

3 years ago
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

3 years ago
Confirmed. All branches are affected due to bug 968576.
Assignee: general → create-and-change
Severity: normal → major
Status: UNCONFIRMED → NEW
Component: Bugzilla-General → Creating/Changing Bugs
Depends on: 968576
Ever confirmed: true
Flags: blocking4.4.4+
Flags: blocking4.2.9+
Flags: blocking4.0.13+
Keywords: regression
Summary: URIs not quoted in Bugzilla/Template.pm:quoteUrls() → URLs pasted in comments are no longer displayed
Target Milestone: --- → Bugzilla 4.0

Updated

3 years ago
Blocks: 998391
(Assignee)

Comment 4

3 years ago
Created attachment 8409068 [details] [diff] [review]
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
Attachment #8409068 - Flags: review?(LpSolit)

Comment 5

3 years ago
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}.
Attachment #8409068 - Flags: review?(LpSolit) → review-
(Assignee)

Comment 6

3 years ago
Created attachment 8409099 [details] [diff] [review]
998323_2.patch

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

dkl
Assignee: create-and-change → dkl
Attachment #8409068 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8409099 - Flags: review?(LpSolit)
(Assignee)

Updated

3 years ago
Attachment #8409099 - Attachment description: 998323_2.patch → Patch for 4.0
Attachment #8409099 - Attachment filename: 998323_2.patch → 998323_40_2.patch
(Assignee)

Comment 7

3 years ago
Created attachment 8409112 [details] [diff] [review]
Patch for 4.2, 4.4, trunk
Attachment #8409112 - Flags: review?(LpSolit)
(Assignee)

Comment 8

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

Need to change comment from discussion in IRC
Attachment #8409099 - Attachment is obsolete: true
Attachment #8409099 - Flags: review?(LpSolit)
(Assignee)

Comment 9

3 years ago
Created attachment 8409114 [details] [diff] [review]
Patch for 4.0
Attachment #8409114 - Flags: review?(LpSolit)

Comment 10

3 years ago
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
Attachment #8409099 - Attachment description: Patch for 4.0 → 998323_2.patch
Attachment #8409099 - Attachment filename: 998323_40_2.patch → 998323_2.patch
Attachment #8409099 - Attachment is obsolete: false

Updated

3 years ago
Attachment #8409099 - Attachment is obsolete: true

Comment 11

3 years ago
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.
Attachment #8409114 - Flags: review?(LpSolit) → review+

Comment 12

3 years ago
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.
Attachment #8408965 - Attachment is obsolete: true
Attachment #8408965 - Flags: review-

Comment 13

3 years ago
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
Attachment #8409112 - Flags: review?(LpSolit) → review+

Updated

3 years ago
Flags: approval?
Flags: approval4.4?
Flags: approval4.2?
Flags: approval4.0?
Flags: approval?
Flags: approval4.4?
Flags: approval4.4+
Flags: approval4.2?
Flags: approval4.2+
Flags: approval4.0?
Flags: approval4.0+
Flags: approval+
(Assignee)

Comment 14

3 years ago
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   406dcbb..f409dc5  master -> master

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   3f79ec4..af76027  4.4 -> 4.4

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   b909fa7..12f4d73  4.2 -> 4.2

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   aa71c23..1f44ef2  4.0 -> 4.0
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.