133406 - errors if quotes in short_desc

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[John Vandenberg]

show_bug.cgi does not filter quotes in the bug short_desc, resulting in invalid
html.

Comment 1

[John Vandenberg]

Attachment: patch

Comment 2

[Myk Melez [:myk] [@mykmelez]]

I believe FILTER html or FILTER url will do this.

Comment 3

[John Vandenberg]

I tried those, with html doing nothing, and with uri, spaces become %20 and
quotes become %22

Comment 4

[John Vandenberg]

From http://www.template-toolkit.org/docs/plain/Manual/Filters.html#html

"html

Converts the characters '<', '>' and '&' to '&lt;', '&gt;' and '&amp',
respectively, protecting them from being interpreted as representing HTML tags
or entities. "

Hence no effect in this case.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

Newer versions of the Template Toolkit's HTML filter do convert quotes to
", and the next release of Bugzilla will probably require version 2.07 when
it is shortly released.

Comment 6

[John Vandenberg]

Strangely enough, v2.06 does as well if I am reading Filter.pm:280 correctly.

            s/"/"/g;

... yet it is not working for me.

Comment 7

[Bradley Baetz (:bbaetz)]

We override the html filter to use html_quote from CGI.pl, which doesn't escape
quotes.

myk, should we just remove that override? It only affected a development version
of TT.

of course, html_quote should handle ", too...

Comment 8

[Andreas Franke (gone)]

See also bug 133425. Sorry, this one was first, so strictly speaking that one
should depend on this one...

Comment 9

[Myk Melez [:myk] [@mykmelez]]

We can't remove the override until TT releases a stable version with the proper
behavior.  This should happen in the near future (2.07 is on a release track). 
In the meantime, we should hack html_quote to do quotation marks as well.

Comment 10

[Andreas Franke (gone)]

Attachment: escape quotes in sub html_quote in CGI.pl

Looking for reviews & checkin then...

Comment 11

[Bradley Baetz (:bbaetz)]

myk: Why not? The current stable release (2.06) wasn't affected, I thought.

Comment 12

[Dave Miller [:justdave]]

I swapped the dependencies back because this is included on the patch on the
other bug, so when that gets checked in, this will automatically get fixed.

Comment 13

[Gervase Markham [:gerv]]

This got fixed incidentally in other show_bug cleanup work. So now either our
html_quote does the right thing, or TTs will when we switch back to that.

Gerv