Open Bug 110769 Opened 23 years ago Updated 2 years ago

nsLocalFileUnix: somewhat improper use of realpath

Categories

(Core :: XPCOM, defect, P3)

x86
Linux
defect

Tracking

()

People

(Reporter: tenthumbs, Unassigned)

References

(Blocks 1 open bug)

Details

nsLocalFile::Normalize uses the realpath system call but doesn't
properly consider the possible errors. In particular, realpath will
return -1 and errno will be ENOENT if the input path does not exist, but
it will also return ENOENT for a dangling symlink. Since all users are
explicitly allowed to create dangling symlinks, this needs to be fixed.

If it were me, I would test for ENOENT and, if the input is a symlink,
just copy the input to the resolved path. Yes, realpath will return a
partially resolved path but that's not very useful since you can't know
which parts were resolved without separating the input into its
components and trying each one in succession. Generally not worth the
trouble.

I suspect, however, that mozilla has some wrong-headed idea about
symlinks so this simple fix won't work.
comments from unix hackers?
Target Milestone: --- → Future
That plan sounds reasonable.  (I recall not wanting ::Normalize to resolve
symlinks at all, but I don't remember the details.)
Yeah, I don't remember much about this either. ( Did I write that code? )
shaver: if you want Normalize to eliminate /../ components, then it must
consider directory symlinks.

tenthumbs: agreed on leaving dangling symlinks denormal and as input.

/be
I think you just want to copy the input to the output on any ENOENT
case, a name that doesn't exist is still a name. There are problems,
though. An un-normalized name can be longer than PATH_MAX so buffer
overruns are a possibility. Other code seems to use MAXPATHLEN so this
may be a real problem, or maybe not.

Are there any specs on what mozilla expects from these methods? They all
appear to be rather ad hoc solutions.

BTW, shouldn't the internal buffer be
  char resolved_path[PATH_MAX + 1] = "";
and after realpath
  resolved_path[PATH_MAX] = '\0'; ?
Assignee: dougt → nobody
QA Contact: scc → xpcom
Assignee: nobody → benjamin
Priority: -- → P2
Target Milestone: Future → ---
Please note bug 349094 comment 3.  I posted a wild-guess patch there for Mac, as I did not have a Linux box at the time.  I would like to see some progress on this.  :-)
Assignee: benjamin → nobody
(In reply to comment #5)
> Are there any specs on what mozilla expects from these methods?

There are specs in nsI(Local)?File.idl, of course, but they're often so vague as to be useless.  We can improve where possible without changing behaviors clients have come to expect, but I hold no hope for having sane, completely specified and implemented file classes prior to Mozilla 2.  I tend to think this case is just one example, and I'd be hesitant to fix it before Mozilla 2.
Or, I suppose more correctly, it's not worth fixing it before then, because it provides a guarantee that isn't all that useful with respect to the rest of nsILocalFile, when one takes into account how inconsistent and unspecified nsILocalFile is.
Yeah, localfile is on my mozilla2-whack list: in particular, if we're going to keep these heavyweight file objects (instead of string paths), we should make them immutable.
Blocks: symlink
Priority: P2 → P3
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.