Closed
Bug 56779
Opened 24 years ago
Closed 24 years ago
Filesystem Datasource doesn't show full directory contents
Categories
(Core Graveyard :: RDF, defect, P2)
Tracking
(Not tracked)
VERIFIED
FIXED
M18
People
(Reporter: bryner, Assigned: bryner)
References
Details
(Keywords: dataloss, Whiteboard: [rtm++] Back to second patch, r=rjc, sr=shaver)
Attachments
(3 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
If the filesystem datasource encounters a dangling symlink while getting a
directory's contents, it stops processing the directory altogether, and files
appearing after that are just skipped.
This is causing several users problems with the filepicker, because there may be
dangling symlinks in system directories which they have no control over.
I have a patch which silently discards the bad file and moves on, instead of
terminating.
Assignee | ||
Comment 1•24 years ago
|
||
Nominating for rtm based on the aforementioned symptoms in the filepicker. The
fix (which I'm attaching next) is extremely safe (my only potential concern
would be a memleak, but I don't think there is one).
Keywords: rtm
Assignee | ||
Comment 2•24 years ago
|
||
Comment 3•24 years ago
|
||
The REAL problem here is that nsFile et.al. should NOT be trying to resolve
symlinks. The bogus symlink should still be shown (and NOT resolved) instead of
being skipped. The RDF file system datasource actually indicates that the item
should not be resolve but nsFile does anyway... that's the real bug, and what
should be fixed. Giving this bug over to dougt...
Assignee: rjc → dougt
Comment 5•24 years ago
|
||
bryner: If you really need a quick fix for 6.0 RTM, a slightly better fix would
be to just completely ignore any errors from the IsDirectory() method call...
and perhaps add a comment as to the reason why errors are being ignored. (Its a
better fix as it would get the broken symlink to appear as well.)
Assignee | ||
Comment 6•24 years ago
|
||
back to rjc so we can try to get a fix for 6.0 in, in the datasource.
Assignee: dougt → rjc
Assignee | ||
Comment 8•24 years ago
|
||
Assignee | ||
Comment 9•24 years ago
|
||
ok, per conversation with rjc, this latest patch goes ahead and shows the
dangling symlink (it doesn't do anything with the error code from IsDirectory).
Status: NEW → ASSIGNED
Comment 10•24 years ago
|
||
Patch #2 looks good to me, r=rjc
Comment 11•24 years ago
|
||
Problems affecting only a few users won't make it into N6 at this point. What
are the circumstances under which this occurs? Expected frequency?
This is not an uncommon occurance, and bryner is right: it sometimes happens
where users have no control over the symlink in question.
I'd be surprised if you found a well-used Unix system that didn't have a few
dangling symlinks -- especially in users' home directories, which is where
they're most likely to be for attaching files, etc.
Comment 13•24 years ago
|
||
Every *nix user is bound to come across tbis bug. The use of symlinks is
extensive on those OS'es. See bug 56760 for a real live sample; users can't play
realplayer as a helper app from it's default location. (And they sure can't use
it as a plugin, in most cases)
What's worse: How can i trust Mozilla not to overwrite files, when it doesn't
even realize files preexist? No alert would appear. Unknowing users risk
destroying files they otherwise would have made backups of. Now THAT is
admittedly a scenario not likely to happen too often, but it will happen. Doing
anything with files under *nix would be like working blindfolded, if this isn't
fixed. Fairly insecure.
Comment 14•24 years ago
|
||
rtm need info, not a top crash, but sounds like a top failure of the app, with a
trivial bullet-proofing fix.
Priority: P3 → P2
Whiteboard: [rtm need info]
Target Milestone: --- → M18
Updated•24 years ago
|
Whiteboard: [rtm need info] → [rtm+ need info]
Comment 15•24 years ago
|
||
Adding dataloss keyword to cover the situation where a user accidently
overwrites one of his/her own files because s/he didn't know it existed.
Keywords: dataloss
Comment 16•24 years ago
|
||
We have an r=, we need an sr=
I'm working on a fix that feels better: make nsLocalFileUnix honour the
.followLinks setting, like the other platforms.
Updated•24 years ago
|
Comment 19•24 years ago
|
||
r=blizzard
Updated•24 years ago
|
Whiteboard: [rtm+ need info] → [rtm+ need info] r=blizzard, need sr=
Comment 20•24 years ago
|
||
Comment 21•24 years ago
|
||
is a r=,a= enough for a rtm+?
Whiteboard: [rtm+ need info] r=blizzard, need sr= → [rtm+ need info] r=blizzard, a=brendan, need sr=?
Comment 22•24 years ago
|
||
I use a= as I did when waterson and I were the only "super-reviewers", and only
for non-netscape.com contributors. It's the same as sr= in my book, although
for some reason, despite years of r= usage and months of a= usage, people think
a= means something else.
This change should get checked into the trunk now that it has passed review.
/be
Whiteboard: [rtm+ need info] r=blizzard, a=brendan, need sr=? → [rtm+]
Comment 23•24 years ago
|
||
Marking [rtm need info]. Does shaver's patch have the same effect as the
previous ones? Should rjc or bryner review? It's not clear to me that everyone
is in agreement; please mark [rtm+] again when The Right Thing is clear.
Updated•24 years ago
|
Whiteboard: [rtm+] → [rtm need info]
Comment 24•24 years ago
|
||
shaver's patch helps out Unix regarding symlink resolution; Windows and Mac
still need the same luv'in...
Assignee | ||
Comment 25•24 years ago
|
||
shaver's patch will have the same net effect in this particular case, but it
does a much better job of fixing the problem at the source.
I'll add my r= to the list here, the patch looks fine to me.
Marking [rtm+] for reconsideration.
Whiteboard: [rtm need info] → [rtm+]
Oh! I had thought, based on the Platform setting and the continual use of
``symlink'' that this was an nsLocalFileUnix-only problem.
I don't know enough about Windows and Mac code here to really say how to fix
them. Maybe the right thing, in light of that, is to take the
datasource-specific fix.
Comment 27•24 years ago
|
||
Ok, so if we're going back to bryner's second patch, that was reviewed by rjc
but we still need a super reviewer, and I'd like evidence that it actually works
on Windows, Mac and Linux.
Marking [need info] again. Sorry if that's a pain, but you wouldn't believe the
problems I've caused by asking questions but leaving the bug [rtm+]
Whiteboard: [rtm+] → [rtm need info]
Updated•24 years ago
|
Whiteboard: [rtm need info] → [rtm need info] Back to second patch, need a sr=
Comment 28•24 years ago
|
||
The right thing to do for the trunk is to fix .followLinks on all platforms.
Using the quick hack for the branch is fine with me, but don't plaster this over
on the trunk.
sr=shaver on bryner's patch #2: it's a good, minimal fix to the datasource.
Assignee | ||
Comment 30•24 years ago
|
||
back to rtm+ since we have an sr=
Whiteboard: [rtm need info] Back to second patch, need a sr= → [rtm+] Back to second patch, need a sr=
Comment 31•24 years ago
|
||
Changing status whiteboard to reflect status.
PDT, please approve this change. If this fix doesn't make it in, I will
be unable to use NS6 as my daily browser, because displaying all
the filenames in a directory is fundimental, there should never be a case
when an application does not show all of the files in a directory.
Furthermore, This bug causes a whole bunch of other bugs, like
the "realplayer plugin problem".
Whiteboard: [rtm+] Back to second patch, need a sr= → [rtm+] Back to second patch, r=rjc, sr=shaver
Comment 32•24 years ago
|
||
rtm++ for "patch #2"
Whiteboard: [rtm+] Back to second patch, r=rjc, sr=shaver → [rtm++] Back to second patch, r=rjc, sr=shaver
Assignee | ||
Comment 33•24 years ago
|
||
checked in on branch and trunk. we should file a new bug for making followLinks
work.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Comment 34•24 years ago
|
||
verified on branch
Linux rh6 1024 build
needs verified on trunk
Keywords: vtrunk
Comment 35•24 years ago
|
||
This may have spawned bug http://bugzilla.mozilla.org/show_bug.cgi?id=57905.
Comment 36•24 years ago
|
||
verified trunk:
Linux rh6 2000123106
Status: RESOLVED → VERIFIED
Keywords: vtrunk
Comment 37•24 years ago
|
||
*** Bug 63719 has been marked as a duplicate of this bug. ***
Updated•6 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•