Re: canonicalize-lgpl bug

[Jim Meyering]

Eric Blake wrote:
> Jim Meyering <jim <at> meyering.net> writes:
>
>> > I noticed that glibc has applied some fixes to canonicalize.c that we were
>> > missing; and sure enough, was able to enhance the testsuite to trip these
>> > bugs.  They also applied some assert's proving that their optimization to
> avoid
>> > strcpy are safe; I left the asserts out, but copied their optimization.  OK
> to
>> > commit?
>>
>> Sure.
>> Thanks for doing that!
>
> Here's the current state of the series.  It still doesn't fix the bug that
> canonicalize-lgpl goofs on leading // when providing realpath.  Nor does it 
> fix
> execution on mingw (where a good implementation need not worry about symlinks,
> but should worry about drive letters and should convert '/' to '\\').  But at
> least on cygwin, canonicalize-lgpl now honors // by virtue of deferring to the
> working native realpath.  On the other hand, cygwin realpath() behaves like 
> all
> other cygwin functions, such as stat() and open(), where garbage/.. is 
> silently
> collapsed whithout checking whether garbage exists or is a symlink.
>
> Note that I've changed things around a bit - canonicalize now provides ONLY
> canonicalize_filename_mode - the coreutils extension that allows for choosing
> how strict the canonicalization needs to be (useful for implementing readlink
> (1)).  Meanwhile, canonicalize-lgpl is the ONLY provider of
> canonicalize_file_name, and it ALSO provides a POSIX-compliant realpath, for
> all systems (well, if there are any live glibc systems out there where
> canonicalize_file_name("file/") still passes rather than failing with ENOENT,
> then we need a subsequent patch to introduce rpl_canonicalize_file_name).
>
> Does anyone want further changes, such as to the names of these modules?  I
> could reasonably see renaming canonicalize to canonicalize-mode, and
> canonicalize-lgpl to either canonicalize_file_name or to realpath.  I could
> also try porting // handling to canonicalize-lgpl realpath (and propose the
> same patch upstream to glibc), although I'd rather not do that until we find a
> platform that honors // but does not have working realpath.
>
> At any rate, I don't feel comfortable pushing this series until I have some
> reviews or more feedback, and possibly a fix to cygwin to better handle '..'.
>
> Jim, once I apply this series, coreutils will need to either: start importing
> canonicalize-lgpl (right now, it avoids it), or change df.c to use
> canonicalize_filename_mode(,CAN_EXISTING) (perhaps via a #define in system.h).
>
> Eric Blake (6):
>       canonicalize-lgpl: reject non-directory with trailing slash
>       stdlib: sort witness names
>       canonicalize: leave canonicalize_file_name to canonicalize-lgpl
>       canonicalize-lgpl: use native realpath if it works
>       canonicalize: don't lose errno
>       canonicalize: honor // if distinct from /

Sounds promising.  Thanks!

I'll read the above more carefully
and take a look at the changes on Monday or Tuesday.