Re: Behavior of file test operations on symlinks

[Andrei Borzenkov <arvidjaar@gmail.com>]

17.02.2016 23:19, Alan Dunn пишет:
> On Wed, Feb 17, 2016 at 8:42 AM, Andrei Borzenkov <arvidjaar@gmail.com>
> wrote:
> 
>> 17.02.2016 03:34, Alan Dunn пишет:
>>> Hi folks,
>>>
>>> Apologies if the following has already come up on this list; I looked for
>>> it and could not find any mention of it.
>>>
>>> I noticed that in a GRUB script "[ -f <dangling symlink path> ]"
>> evaluates
>>> to true.  This is unlike the behavior of the "test" binary, in which it
>>> returns false: most file test operations dereference their symlinks
>>> recursively (i.e., strace on Linux reveals they use stat, which does
>>> this).  By contrast, "[ -s <dangling symlink path> ]" evaluates to false,
>>> which seems inconsistent since if the file exists by -f, then it seems
>> like
>>> -f is referring to the symlink itself, which has non-zero file size.
>>>
>>
>> It looks rather side effect of implementation which looks for directory
>> entry.
>>
>> It is straightforward to fix it by just trying to grub_file_open() which
>> fails in this case. But the interesting question is semantic of both
>> tests with mandatory signature checking in place. I.e. if signature for
>> a file is invalid, should "test -s" and "test -f" still report true? I
>> suppose yes, because file still exists.
>>
>>> I was curious whether there is some motivation with respect to any
>>> deviations that GRUB has in interpreting file test operations in
>> comparison
>>> to the "test" binary, or whether this is considered a bug/thing that
>> should
>>> be improved in the documentation.  The GRUB manual (
>>> http://git.savannah.gnu.org/cgit/grub.git/tree/docs/grub.texi) only
>>> indicates that -f tests whether the "file exists and is not a directory"
>>
>> Well, current behavior is compliant with this description (symlink does
>> exist and it is not directory), it is just not very useful in practice.
>> Actually implementing "test -h" is pretty trivial.
>>
>>> without specifying the symlink behavior (unlike "man test").
>>>
>>
>> I vote for changing it to follow symlink. Anyone has argument to keep
>> current behavior?
>>
> 
> If I were going to design GRUB's behavior, I would make all file-related
> tests that GRUB implements (-d, -e, -f, -s, -nt, -ot) follow symlinks, like
> test in coreutils does, for consistency.  If that sounds reasonable to the
> list, I'm happy to try to produce such a patch.  Offhand, it seems like the
> basic strategy would be to modify get_fileinfo (in
> grub-core/commands/test.c) to follow symlinks.
> 

It means reimplementing symlink traversal yet again and doing it pretty
inefficiently. Storing mtime in struct grub_file and using
grub_file_open seems to be the least evil.