Closed Bug 183871 Opened 22 years ago Closed 21 years ago

Crash on "Show hidden files and directories" [@ nsFileView::SetDirectory(nsIFile*) ]


(Core Graveyard :: File Handling, defect)

Not set


(Not tracked)



(Reporter: relf, Assigned: Biesinger)



(Keywords: crash)

Crash Data


(2 files, 3 obsolete files)

Linux build 2002120508

To reproduce:
1. Open Edit->Preferences...->Advanced->Cache->Choose Folder
2. Click on "Show hidden files and directories" checkbox
3. Mozilla crashes
That happens when "Choose Cache Folder" dialog says:
"You do not have the permissions necessary to view this directory".
can you post Talkback ID or stack trace ?
Keywords: crash, stackwanted
How to get them?
are you using a build?  this bug has been reported multiple times,
all by users of debian builds except for one running RPMS from Red Hat.  see bug
Max, for the Talkback part, see "mozilla/bin/components/talkback/talkback" give
you actually submitted Feedback when the Netscape Feedback agent popped up after
Mozilla crashed.
yeah. i'm using build

I don't see talkback subdirectory in mozilla/conponents.
The bug is easily reproducible here. Just let me know which build/tool to use to
provide you necessary information.
talkback is not included in the .tar.gz builds.  You need to get the "sea" or
"installer" build.
Attached file Talkback info (obsolete) —
we just need the talkback ID.  run mozilla/components/talkback/talkback and grab
the talkback ID for the report you sent in.
I didn't get how to learn ID.
I click 'Send' in Talkback agent, it disappears, and nothing happen.
> I click 'Send' in Talkback agent, it disappears, and nothing happen.

that's fine.  now go to the mozilla directory (i.e. /usr/local/mozilla) and then
go to components/talkback.  run the talkback executable in that directory.

in an attempt to know why you're seeing this when nobody else is... what Mozilla
theme are you using?  what gtk theme are you using?
Ok. Talkback ID: TB14817508W

I see the bug with any theme including default Modern one.
I'm using Debian release 3.0 - dunno what gtk it comes with.
Attachment #108541 - Attachment is obsolete: true
Whiteboard: TB14817508W
Blocks: 185584
No longer blocks: 185584
we need a new TB ID, this one has expired (30 days).
Whiteboard: TB14817508W
Ok. This is a fresh one.
Linux build 2003010610
Keywords: stackwanted
Whiteboard: TB15870027Y
Trigger Reason  	SIGSEGV: Segmentation Fault: (signal 11)
Keywords: stackwanted
Whiteboard: TB15870027Y
bug 176886 strikes again.
Olivier, do you want a new stack again?
From which build?
Max, we'd need a stack from GDB with a debug build you'd compile yourself. I
can't reproduce this on my Linux box and Talkback sometimes totally misses the
you could also try running with a clean profile.  if there's something in your
profile that's triggering the crash, we'd need to know what it is to reproduce
the crash.
Clean profile works fine. And I've found what causes the crash.

I have two primary OS installed: OS/2 and Linux. And I'm trying to share Mozilla
settings between them that is objected by the bug 137006. So I have to use some
tricks to pass over that bug (at least partially). Mozilla settings are located
on OS/2 drive O: which is mounted as /O: directory in Linux.
Well... The current crash problem arises when I use profile from OS/2 version
containing line like


Hope, this will help.
thanks, Max.  I can reproduce it with that setting.

marking NEW.  it's an invalid pref setting, but Mozilla shouldn't crash.
stacktrace to follow.
Ever confirmed: true
Keywords: stackwanted
Attached file stacktrace
(gdb) frame 7
#7  0x41ff39d3 in nsFileView::SetDirectory(nsIFile*) (this=0x8878488,
aDirectory=0x1) at nsFileView.cpp:244
244	  aDirectory->GetDirectoryEntries(getter_AddRefs(dirEntries));
(gdb) p aDirectory
$1 = (nsIFile *) 0x1
(gdb) p *aDirectory
Cannot access memory at address 0x1
(gdb) frame 8
#8  0x41ff37c2 in nsFileView::SetShowHiddenFiles(int) (this=0x8878488,
    at nsFileView.cpp:158
158	    SetDirectory(mDirectoryPath);
(gdb) p mDirectoryPath
$2 = {mRawPtr = 0x0}
to bryner, since he's most familiar with this code....
Assignee: law → bryner
BTW, i disagree that pref is invalid. In Linux it can be treated as a valid
directory whose name contains '\' and ':' symbols. Nothing wrong with that.
this kind of this is supposed to be handled (see bug 105809).
162   if (!directory || !(sfile.exists() && sfile.isDirectory())) {
163     // Start in the user's home directory
164     sfile.initWithPath(homeDir.path);
165   }

sfile.exists() returns false, but it still checks sfile.isDirectory(), which
throws an unhandled Javascript exception, so the directory never gets reset.
Attached patch patch (obsolete) — Splinter Review
just check exists() and isDirectory() separately.
umm.... if sfile.exists() returns false (are you sure that it does?) then it
checking there is a blatant bug in JS, which is defined to do
operator short-circuiting....

Are you sure that's what's going on?
Attachment #111325 - Flags: review?(bryner)
Comment on attachment 111325 [details] [diff] [review]

it appears that the exception was from some debugging I had, but the patch did
fix things.  However, backing out the patch now also works.  I'm confused.
Attachment #111325 - Flags: review?(bryner)
Comment on attachment 111325 [details] [diff] [review]

ok, to reproduce this bug, you have to actually visit a page so that it creates
the bogus cache directory (relative to the current directory).	If the
directory does not exist, then the filepicker behaves as advertised and falls
back to the home directory.
Attachment #111325 - Attachment is obsolete: true
ok, the real problem is that filepicker's populateAncestorList() accesses
directory.parent, which returns NS_ERROR_FILE_INVALID_PATH because the path does
not contain a '/':
GetParent seems to expect an absolute path, so a path without a '/' is
inherently invalid.

try/catching the access to directory.parent and the while loop seems to fix
things up a bit.  The directory is properly displayed in the filepicker and
turning on hidden files doesn't crash, but (of course) the "Look in" dropdown is
unpopulated and hitting "Up" doesn't do anything.
Ugh.  So yes, getComplexValue from prefs will call setPersistentDescriptor with
whatever garbage is in the pref.  And nsLocalFileUnix will cheerfully let itself
be initialized (through InitWithNativePath()) with a path that has nothing to do
with a Unix path... even though it later totally fails to deal (eg see assertion
in GetParent, the logic in AppendRelativeNativePath, etc, etc).

Adding some exception catching code to the filepicker is fine with me, but only
if .parent is allowed to throw.  Too bad we didn't bother documenting that in

Should InitWithNativePath throw when the path is obviously bogus?
Initializing nsLocalFileUnix w/ a bogus value sounds like bad client logic. If
nsLocalFileUnix is crashing, that is a different story.

I tried to reproduce on debian, but no crash.

Um... the client (prefs) initializes an nsILocalFile.  It does not know what the
underlying impl is and it should not care, since it makes no sense to put the
validity checks in _all_ the clients when those would be much more cleanly
encapsulated in the nsILocalFile implementations.

More to the point, nsLocalFileWin, nsLocalFileMac, nsLocalFileOS2,
nsLocalFileOSX,  all perform sanity-checking on the path and will return
NS_ERROR_UNRECOGNIZED_PATH if the path is not recognized (makes sense, eh?). 
Yeah, every single client of nsILocalFile could include the
couple-of-hundred-lines of ifdeffed error checking... do you really think that
is the right approach?

No, nsLocalFileUnix does not crash.  It will simply throw exceptions that the
interface does not define as being thrown and behave in completely ridiculous
ways in other cases....  It seems to me, that it would make more sense to throw
an exception where callers are actually expecting one and prepared to deal...
Would fixing this be as easy as adding a check for file existance in
You can created nsIFiles pointing to non-existing places and then call Create()
or CreateUnique() on them, you know...  Breaking that would be BAD (frozen
interface, yadda yadda).  ;)
how about this, can we make the unix localfile implementation throw an exception
if Init is called with a relative path?
~/ should still be supported.
Blocks: 219107
Summary: Crash on "Show hidden files and directories" → Crash on "Show hidden files and directories" [@ nsFileView::SetDirectory(nsIFile*) ]
well... unfortunately this patch breaks xpcshell, which tries to init nsIFiles
with "."
Attached patch patchSplinter Review
well the real problem was that nsDirectoryService.cpp was relying on the fact
that nsLocalFileUnix::InitWithNativePath accepted relative paths. That
contradicted its interface documentation, but oh well...
Attachment #137189 - Attachment is obsolete: true
Assignee: bryner → file-handling
QA Contact: chrispetersen → ian
Target Milestone: --- → mozilla1.7alpha
really taking this time
Assignee: file-handling → cbiesinger
Comment on attachment 137198 [details] [diff] [review]

Is there a nsDirectoryService.cpp patch?

Why can't the value of PR_GetEnv(...) be a canonicalized absolute path and drop
this realpath business?  

I think that your change to xpcom/io/nsLocalFileUnix.cpp may break things. 
Yes, the interface says one thing but as you pointed out, the callers do
another.  What kind of testing have you done?
>Is there a nsDirectoryService.cpp patch?

yes, in the exact attachment you commented on

>Why can't the value of PR_GetEnv(...) be a canonicalized absolute path and drop
>this realpath business? 

let me note that GRE_GetCurrentProcessDirectory already uses the realpath solution.

>I think that your change to xpcom/io/nsLocalFileUnix.cpp may break things. 

Well, nsLocalFileUnix seems to not always deal with that either - GetLeafName
seems to require absolute paths. Also GetParent.

>What kind of testing have you done?

run xpcshell, run mozilla, do some browsing, run mailnews, attach patches (using
the filepicker).
do you have anything specific in mind that you suspect of breaking?
Comment on attachment 137198 [details] [diff] [review]

lets get this into 1.7a.
Attachment #137198 - Flags: review?(dougt) → review+
Attachment #137198 - Flags: superreview?(bz-vacation)
Comment on attachment 137198 [details] [diff] [review]

Attachment #137198 - Flags: superreview?(bz-vacation) → superreview+
ok, checked in
Checking in nsDirectoryService.cpp;
/cvsroot/mozilla/xpcom/io/nsDirectoryService.cpp,v  <--  nsDirectoryService.cpp
new revision: 1.67; previous revision: 1.66
Checking in nsLocalFileUnix.cpp;
/cvsroot/mozilla/xpcom/io/nsLocalFileUnix.cpp,v  <--  nsLocalFileUnix.cpp
new revision: 1.109; previous revision: 1.108
Closed: 21 years ago
Resolution: --- → FIXED
(In reply to comment #43)
> I think that your change to xpcom/io/nsLocalFileUnix.cpp may break things.

Just for the record, dougt was right and this change did cause bug 238290.
Crash Signature: [@ nsFileView::SetDirectory(nsIFile*) ]
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.