Closed Bug 779057 Opened 12 years ago Closed 12 years ago

DirInstallLocation._readAddons() deletes stale pointer files unsafely

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: Unfocused, Assigned: capella)

Details

(Whiteboard: [good first bug][mentor=bmcbride@mozilla.com][lang=js])

Attachments

(1 file, 1 obsolete file)

DirInstallLocation._readAddons() will delete a pointer file if it finds its stale (ie, not valid). However, it does so unsafely - not checking if it's able to remove the file, and not catching any exceptions if deleting the file fails. It should wrap the .remove() in a try/catch block, so that if it does fail, things can just continue on normally.

The code in question is:
http://hg.mozilla.org/mozilla-central/file/99c53832abfe/toolkit/mozapps/extensions/XPIProvider.jsm#l6305
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
Attached patch Patch (v1) (obsolete) — Splinter Review
Attachment #649625 - Flags: review?(bmcbride)
Comment on attachment 649625 [details] [diff] [review]
Patch (v1)

Review of attachment 649625 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/extensions/XPIProvider.jsm
@@ +6305,5 @@
> +          try {
> +            entry.remove(true);
> +          }
> +          catch (e) {
> +            WARN("Failed to remove stale pointer file", e);

Can you add the filename to this message? I know its a bit redundant, but by default you'd only see the warning (LOG only outputs anything if extensions.logging.enabled=true).
Attachment #649625 - Flags: review?(bmcbride) → review-
Attached patch Patch (v2)Splinter Review
Easy enough!
Attachment #649625 - Attachment is obsolete: true
Attachment #649632 - Flags: review?(bmcbride)
Comment on attachment 649632 [details] [diff] [review]
Patch (v2)

Review of attachment 649632 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks :)
Attachment #649632 - Flags: review?(bmcbride) → review+
https://hg.mozilla.org/integration/fx-team/rev/ba1293500482
Flags: in-testsuite-
Flags: in-qa-testsuite-
Flags: in-moztrap-
Flags: in-litmus-
https://hg.mozilla.org/mozilla-central/rev/ba1293500482
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: