Last Comment Bug 691226 - about:memory can fail due to generating invalid XML
: about:memory can fail due to generating invalid XML
Status: VERIFIED FIXED
[qa!]
: verified-aurora, verified-beta
Product: Toolkit
Classification: Components
Component: about:memory (show other bugs)
: Trunk
: x86 Mac OS X
: P2 normal (vote)
: mozilla10
Assigned To: Boris Zbarsky [:bz] (Out June 25-July 6)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-02 19:32 PDT by Boris Zbarsky [:bz] (Out June 25-July 6)
Modified: 2011-11-28 07:53 PST (History)
7 users (show)
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
proposed fix (1.07 KB, patch)
2011-10-02 20:18 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
justin.lebar+bug: review+
asa: approval‑mozilla‑aurora+
Details | Diff | Review

Description Boris Zbarsky [:bz] (Out June 25-July 6) 2011-10-02 19:32:19 PDT
When I open about:memory?verbose in my build right now, I get a blank page and an XML well-formedness error in the error console.  The culprit is this:

<span class='mrName hasDesc' title='The sum of all entries below &#39;shell(javascript:&#39;<html></html>&#39;)&#39;.'>shell(javascript:&#39;&lt;html&gt;&lt;/html&gt;&#39;)</span>

'<' is not allowed in attribute values in XML.

The relevant part of aboutMemory.js is this:

852   var text = "-- <span class='mrName hasDesc' title='" +
853              kindToString(aKind) + prepDesc(aDesc) +
854              "'>" + prepName(aName) + "</span>";

where prepName escapes '&', '<', '>', '\'', and '"' while prepDesc only escapes '&' and '\''.  At the very least, prepDesc needs to escape '<'.  But is there any reason at all to not use prepName in both places?  Not posting a patch for now on the off chance that I'm missing something here...
Comment 1 Justin Lebar (not reading bugmail) 2011-10-02 19:58:17 PDT
I don't think you're missing something...
Comment 2 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-10-02 20:18:03 PDT
Created attachment 564110 [details] [diff] [review]
proposed fix

I left the separate prepName/prepDesc functions in case we actually want to do more processing on one of them sometime.  Let me know if you'd prefer they just be merged.
Comment 3 Justin Lebar (not reading bugmail) 2011-10-03 10:38:51 PDT
Comment on attachment 564110 [details] [diff] [review]
proposed fix

r=me, but please remove escapeQuotes() -- it's not used, and it sounds like it's not correct under any circumstances!
Comment 4 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-10-03 12:15:04 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/092140ecc44c

Justin, do you think we want this on aurora/beta?
Comment 5 Justin Lebar (not reading bugmail) 2011-10-03 12:27:48 PDT
I don't think it's critical to take on aurora/beta, but it would be nice, since if someone has a memory problem and hits this case, we can't get any data out of them.
Comment 6 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-10-03 12:35:13 PDT
Comment on attachment 564110 [details] [diff] [review]
proposed fix

Requesting aurora approval.  This should be pretty safe, and we need all the memory info we can get.

I'm not going to try to get this into beta.  Too much risk/reward ratio at that point, I think, if the risk is anything but 0.
Comment 7 Matt Brubeck (:mbrubeck) 2011-10-03 16:52:44 PDT
https://hg.mozilla.org/mozilla-central/rev/092140ecc44c
Comment 8 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-10-06 19:46:36 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/6763937c5a1f
Comment 9 Vlad [QA] 2011-11-28 07:53:10 PST
Setting resolution to Verified Fixed on:

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0) Gecko/20100101 Firefox/9.0 beta 3
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0a2) Gecko/20111127 Firefox/10.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0a1) Gecko/20111127 Firefox/11.0a1

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:9.0) Gecko/20100101 Firefox/9.0 beta 3
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:10.0a2) Gecko/20111127 Firefox/10.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:11.0a1) Gecko/20111128 Firefox/11.0a1

The about:memory?verbose page is displayed properly.

Note You need to log in before you can comment on or make changes to this bug.