Closed Bug 232861 Opened 21 years ago Closed 20 years ago

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

Categories

(Bugzilla :: Attachments & Requests, defect, P2)

2.17.6
defect

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: dewildt, Assigned: bugreport)

References

()

Details

Attachments

(2 files)

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.
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: myk → bugreport
Status: NEW → ASSIGNED
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)
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
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 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+
Flags: approval?
looks good to me, too.
Flags: approval? → approval+
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: 20 years ago
Resolution: --- → FIXED
Whiteboard: [applied to b.m.o]
Whiteboard: [applied to b.m.o]
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.

Attachment

General

Created:
Updated:
Size: