nsLocalFileUnix::Remove(PR_TRUE) shouldn't ever follow symlinks

RESOLVED FIXED

Status

()

Core
XPCOM
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: jag (Peter Annema), Assigned: jag (Peter Annema))

Tracking

(Blocks: 1 bug)

unspecified
All
Linux
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

9 years ago
It's not what rm -r does, and it leads to all kinds of unexpected behaviour when the nsIFile is a symlink to a directory: we recursively remove the target directory's entries, and then try to rmdir the symlink.

The fix should be as simple as removing the !recursive check and just always unlinking symlinks.
(Assignee)

Comment 1

9 years ago
Created attachment 368645 [details] [diff] [review]
Always unlink symlinks
Assignee: nobody → jag
(Assignee)

Comment 2

9 years ago
Created attachment 368646 [details] [diff] [review]
Same, diff -w
(Assignee)

Updated

9 years ago
Attachment #368645 - Flags: superreview?(benjamin)
Attachment #368645 - Flags: review?(joshmoz)

Comment 3

9 years ago
Comment on attachment 368645 [details] [diff] [review]
Always unlink symlinks

Code looks fine to me and I'm OK with this behavior.
Attachment #368645 - Flags: review?(joshmoz) → review+

Comment 4

9 years ago
Comment on attachment 368645 [details] [diff] [review]
Always unlink symlinks

Does the other test I reviewed cover this bug as well? This should have tests to land.
Attachment #368645 - Flags: superreview?(benjamin) → superreview+

Updated

9 years ago
Flags: in-testsuite?
(Assignee)

Comment 5

9 years ago
After discussing this with Benjamin on irc, I'll land the test as-is, then remove the work-around when I land this.
(Assignee)

Comment 6

9 years ago
http://hg.mozilla.org/mozilla-central/rev/19126e806902
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.