bug/comment/attachment/urls get expanded in description/title of attachment link in comment (bug in quoteUrls)

RESOLVED FIXED in Bugzilla 2.18

Status

()

defect
P2
critical
RESOLVED FIXED
16 years ago
7 years ago

People

(Reporter: dewildt, Assigned: bugreport)

Tracking

2.17.6
Bugzilla 2.18
Dependency tree / graph
Bug Flags:
approval +
blocking2.18 +

Details

()

Attachments

(2 attachments)

Reporter

Description

16 years ago
In bug 220773 comment 15 is an attachment created with the phrase 'comment ##'
in the description. Bugzilla creates automaticly a link from this phrase. The
html code for the link is shown in the title of the <A> tag of the new created
attachment. The quotation mark in the link closes the quotation mark of the title.
Reporter

Comment 1

16 years ago
bug 220773 comment 17 shows the link correct.
whew.  everything gets quoted at least. :)  Was worried for a minute we had
anothe XSS bug on our hands.
Severity: normal → major
OS: Windows 2000 → All
Hardware: PC → All
Target Milestone: --- → Bugzilla 2.18
*** Bug 234259 has been marked as a duplicate of this bug. ***
See my comments to caillon in the other bug.
*** Bug 234160 has been marked as a duplicate of this bug. ***
*** Bug 234419 has been marked as a duplicate of this bug. ***
Rewriting title to try and make it more easily findable by the people who are
filing dupes :-)

Gerv
Summary: html code in comment if created attachment description contains an automatic generated link → Word "bug" gets expanded in title attribute of attachment link in comment (bug in quoteUrls)
But "bug" isn't the only thing that get's expanded, per the rest of the comments
on this bug, so that's inaccurate.  Changing summary again.
Summary: Word "bug" gets expanded in title attribute of attachment link in comment (bug in quoteUrls) → bug/comment/attachment/urls get expanded in description/title of attachment link in comment (bug in quoteUrls)
*** Bug 234709 has been marked as a duplicate of this bug. ***
Flags: blocking2.18+
bbaetz said:

"I think that we need do do the @things stuff. Replacement text in those regexps
cannot ever expand to somethign that would be matched by a larger regexp.

You can't use @things itself, because then we can't put it back - you need to
use a new array/substitution bit."


What is "the @things stuff"? Is there a decent fix for this for 2.18, or does it
require a total rewrite of that munging code?

Gerv
see @things in the relevent code. The issue is that we need to pul out and then
somehow readd the substitution back in, without doign the double substitution
which is happening now.
*** Bug 239033 has been marked as a duplicate of this bug. ***
Severity: major → critical
Assignee

Updated

15 years ago
Assignee: myk → bugreport
Status: NEW → ASSIGNED
Assignee

Comment 16

15 years ago
Comment on attachment 147967 [details] [diff] [review]
Patch - use @things to prevent double-interpretation

The @things array is a list of numeric placeholders that look like
\0\0number\0\0

At the very end, all the instances of \0\0number\0\0 get replaced with whatever
they were placeholding for.  This is now extended to cover attachment
linkification so that bug and comment linkification (which comes later) does
not see the word bug or comment in attachment autolinks.
Attachment #147967 - Flags: review?(gerv)
Assignee

Updated

15 years ago
Priority: -- → P2
See my comment which gerv quoted:

> You can't use @things itself, because then we can't put it back - you need to
> use a new array/substitution bit.

Hopwever, I can't now remember what the issue was.. I'll have to think about it
Assignee

Comment 18

15 years ago
I cannot see why the same list would be an issue.   After all, count continues
to increment, so...

bug 232861
http://bugzilla.mozilla.org
attachment 147967 [details] [diff] [review]
mailto:justdave@bugzilla.org
http://localhost/bug#232861
bug 1

becomes
bug 232861
\0\00\0\0
\0\03\0\0
\0\01\0\0
\0\0\2\0\0
bug 1

and then substitues back

Neither can I at the moment. Hmm

Comment 20

15 years ago
Comment on attachment 147967 [details] [diff] [review]
Patch - use @things to prevent double-interpretation

>+              ~($things[$count++] = GetAttachmentLink($2, $1)) &&
>+               ("\0\0" . ($count-1) . "\0\0")

I wonder if that syntax could be globally replaced w/ something like:
+	       ~($things[$count] = GetAttachmentLink($2, $1)) &&
+		("\0\0" . ($count++) . "\0\0")

but, that's food for another bug.
Attachment #147967 - Flags: review?(gerv) → review+
Assignee

Updated

15 years ago
Flags: approval?
looks good to me, too.
Flags: approval? → approval+
Assignee

Comment 22

15 years ago
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.266; previous revision: 1.265
done
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [applied to b.m.o]
Whiteboard: [applied to b.m.o]

Comment 23

13 years ago
I'm using bugzilla 2.22 and having the same issue. 
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.