Last Comment Bug 780686 - DOMApplicationRegistry._cloneAppObject doesn't clone the `receipts` array
: DOMApplicationRegistry._cloneAppObject doesn't clone the `receipts` array
Status: RESOLVED FIXED
[qa-]
:
Product: Core
Classification: Components
Component: DOM: Apps (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla17
Assigned To: Myk Melez [:myk] [@mykmelez]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-06 12:32 PDT by Myk Melez [:myk] [@mykmelez]
Modified: 2012-08-12 08:33 PDT (History)
3 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v1: clones `receipts` array (504 bytes, patch)
2012-08-06 12:32 PDT, Myk Melez [:myk] [@mykmelez]
fabrice: review+
myk: checkin+
Details | Diff | Splinter Review

Description Myk Melez [:myk] [@mykmelez] 2012-08-06 12:32:55 PDT
Created attachment 649351 [details] [diff] [review]
patch v1: clones `receipts` array

DOMApplicationRegistry._cloneAppObject doesn't clone the `receipts` array, which it obtains from content.

This causes DOMWindows that install apps to leak due to bug 780674 (which should get fixed by the eventual fix for that bug).  It also makes it possible for a webapp to implicitly modify an app's receipts after installing the app, which seems undesirable.  And, more generally, it potentially misleads callers who expect the method to deeply clone the app object, such that the clone contains no references to parts of the original object.

Here's a fix that uses JSON to clone the array.
Comment 1 Myk Melez [:myk] [@mykmelez] 2012-08-07 13:57:44 PDT
Comment on attachment 649351 [details] [diff] [review]
patch v1: clones `receipts` array

https://hg.mozilla.org/integration/mozilla-inbound/rev/4e54a6eb43d2
Comment 2 [:fabrice] Fabrice Desré 2012-08-07 15:57:21 PDT
Small followup:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fcb650e7bd6e
Comment 3 Ryan VanderMeulen [:RyanVM] 2012-08-07 17:50:52 PDT
Awesome, so this was backed out along with bug 772299 due to mochitest-other permaorange.
https://hg.mozilla.org/integration/mozilla-inbound/rev/5886a528d6db

After backing out, I realized that Fabrice's follow-up fixed the orange. Please be starring builds when pushing bustage fixes...

Re-pushed (with the follow-up included).
https://hg.mozilla.org/integration/mozilla-inbound/rev/922cbdeaaec4

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