DirInstallLocation._readAddons() deletes stale pointer files unsafely

RESOLVED FIXED in mozilla17

Status

()

Toolkit
Add-ons Manager
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Unfocused, Assigned: capella)

Tracking

unspecified
mozilla17
Points:
---
Bug Flags:
in-qa-testsuite -
in-testsuite -
in-litmus -
in-moztrap -

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(1 attachment, 1 obsolete attachment)

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)

Updated

6 years ago
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
(Assignee)

Comment 1

6 years ago
Created attachment 649625 [details] [diff] [review]
Patch (v1)
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-
(Assignee)

Comment 3

6 years ago
Created attachment 649632 [details] [diff] [review]
Patch (v2)

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
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.