109530 - xml.cgi double (and in some cases triple) quoting special characters.

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect, P2)

Description

[Jacob Steenhagen]

This is caused by the fact that Bug.pm runs some fields through QuoteXMLChars()
or html_quote() when it populates its hash and then runs them through
QuoteXMLChars() again when it outputs the XML in emitXML().  Bug.pm's hash of
values should not quote the data in any way.  It should provide "raw" data that
can be quoted depending on the needs of the caller, not just arbitrarily
html/xml quoted.

Also, QuoteXMLChars() has a bug in it in that the order it quotes can cause the
& to get quoted again (making it triple quoted).

Comment 1

[Jacob Steenhagen]

Attachment: fixes known issues

Comment 2

[Jacob Steenhagen]

I'd like to see this get fixed soon as I'm working on making ssdbot use xml.cgi
to retrieve single bugs instead of buglist.cgi (avoiding the shadow database and
funny html parsing madness).

Comment 3

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

Jake, take a look at my patches to buglist.cgi in bug 103778.  They include
support for outputting query results as RDF (a form of structured XML).

Comment 4

[Jacob Steenhagen]

Myk, I remember seeing that :)  I do intend to start working on using that
facility once it's available for the query interface... however, I'll probably
still want to use xml.cgi (or similar) for single bug lookups as it allows me to
not be hindered by the shadow database (esp. for when bzbot reports new bugs and
wants ssdbot to be able to look them up :)

Comment 5

[Gervase Markham [:gerv]]

You've made these changes to remove the quoting - but presumably some consumer
of the info was expecting a quoted version. Do you not need to add these calls
back in somewhere else, e.g. in a template?

Gerv

Comment 6

[Jacob Steenhagen]

Attachment: Bug.pm and importxml.pl

The only place that Bug.pm is used is with regards to the moving code.	This
fixes the other half of moving to not double-unquote (as XML::Parser does the
first unquote).

Comment 7

[Dave Miller [:justdave]]

We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.

Comment 8

[Jacob Steenhagen]

Bringing back down to 2.16... this makes ssdbot's output really ugly for bugs
with special characters in them.  The fix is easy, low risk and really needs to
be done before we can use Bug.pm in anything else (and I think it'd be useful
for the show_bug.cgi templatization).

Comment 9

[Dave Miller [:justdave]]

Comment on attachment 57496 [details] [diff] [review]
Bug.pm and importxml.pl

r= justdave

Comment 10

[Luis Villa]

One quick question about the patch:

 sub QuoteXMLChars {
+  $_[0] =~ s/&/&/g;
   $_[0] =~ s/</&lt;/g;
   $_[0] =~ s/>/&gt;/g;
   $_[0] =~ s/'/&apos;/g;
   $_[0] =~ s/"/&quot;/g;
-  $_[0] =~ s/&/&amp;/g;
 # $_[0] =~ s/([\x80-\xFF])/&XmlUtf8Encode(ord($1))/ge;
   return($_[0]);

is there some reason for the re-ordering, since that is all this appears to be
doing? Otherwise, it looks fine to me.

Comment 11

[Luis Villa]

reordering explained in irc; patch applied/tested on my 2.14 testbed. Looks good. 
r= louie

Comment 12

[Jacob Steenhagen]

Checking in Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.9; previous revision: 1.8
done
Checking in importxml.pl;
/cvsroot/mozilla/webtools/bugzilla/importxml.pl,v  <--  importxml.pl
new revision: 1.20; previous revision: 1.19
done