zsh-workers
 help / color / mirror / code / Atom feed
From: Devin Hussey <husseydevin@gmail.com>
To: Bart Schaefer <schaefer@brasslantern.com>
Cc: zsh-workers@zsh.org
Subject: Re: [PATCH] Allow globbing with unreadable parent directories
Date: Wed, 13 Jan 2021 19:27:51 -0500	[thread overview]
Message-ID: <CAEtFKssJVz9mr3vA_vY77m2bsrXWBKHz1qgv83jzeY0jLXzPjQ@mail.gmail.com> (raw)
In-Reply-To: <CAH+w=7bn8JYPx29LPGnT4ughEt-z+-rUokUz739foP=JCY9B7g@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2592 bytes --]

On Wed, Jan 13, 2021, 5:28 PM Bart Schaefer <schaefer@brasslantern.com>
wrote:

> On Tue, Jan 12, 2021 at 7:04 PM Devin Hussey <husseydevin@gmail.com>
> wrote:
> >
> > POSIX specifies that when globbing, parent directories only have to be
> > searchable, not readable.
> >
> > +       /* First check for search permission. */
> > +       if (access(fn, X_OK) != 0)
> >             return;
>
> I don't think this is correct.  Even if it is strictly correct per
> POSIX (which would seem strange to me), it should not be applied in
> zsh native mode (see my previous email about setopts).
>
> % mkdir /tmp/adirectory
> % touch /tmp/adirectory/somefile
> % chmod a-x /tmp/adirectory
> % echo /tmp/adirectory/*
> /tmp/adirectory/somefile
> % echo /tmp/adirectory/*(.)
> zsh: no match
>
> Lack of search permission only means that you can't tell what kind of
> file "somefile" is (you can't read its inode data, e.g., stat() it),
> not that you can't see the name itself.
>

This matches the behavior of the "pure" globber, POSIX, and literally every
other shell.

If we made it accept it unconditionally like my original solution, we would
end up with the *opposite* issue, where CASE_GLOB would fail on files where
NO_CASE_GLOB succeeds.

Case insensitivity should not change the output due to file permissions.

> +       /* Then, if we have read permission, try to open the directory. */
> > +       if (access(fn, R_OK) == 0) {
> > +           int dirs = !!q->next;
> > +           DIR *lock = opendir(fn);
>
> The access call here is redundant; opendir(fn) will fail if there is
> not read permission.  Why do you believe the extra test is needed?


opendir(fn) will also fail if the "folder" is a file.

However, you may be right, if the path is a file, the next access() will
definitely fail, so I could remove that and only return if we can't
access(fn, X_OK).

I think the rest is just re-indentation.  Did I miss an "else" clause
> for the R_OK ?
>

Yes it is just reindentation.

There is no else clause, it goes to the next section of the path if the
current directory is unreadable.


> Can you provide a specific test case that shows the difference in
> behavior with and without this patch?


I think I can throw something together, yes.

As far as I can tell, the patch

would only cause globbing to fail in more cases, not succeed where it
> previously did not.
>

No, that is definitely not the case.

opendir() would fail if either R_OK or X_OK was false, causing unreadable
folders to be a false negative.

This is allowing certain combinations where opendir() would fail.

>

[-- Attachment #2: Type: text/html, Size: 4979 bytes --]

  reply	other threads:[~2021-01-14  0:28 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-13  3:04 Devin Hussey
2021-01-13 22:27 ` Bart Schaefer
2021-01-14  0:27   ` Devin Hussey [this message]
2021-01-14  1:32     ` Lawrence Velázquez
2021-01-14  2:22       ` Devin Hussey
2021-01-14  2:24         ` Devin Hussey
2021-01-17 17:22       ` Daniel Shahaf
2021-01-17 18:02         ` Bart Schaefer
2021-01-17 18:23           ` Bart Schaefer
2021-01-14  4:04     ` Bart Schaefer
2021-01-14  5:57       ` NO_CASE_GLOB and unreadable directories (Episode VI: A New Hope) Bart Schaefer
2021-01-14 18:56         ` Bart Schaefer
2021-01-25  0:52         ` Bart Schaefer
2021-01-25 14:05           ` Peter Stephenson
2021-02-05  5:53             ` Bart Schaefer
2021-02-06 12:31               ` Daniel Shahaf
2021-03-27 17:31           ` Lawrence Velázquez
2021-04-10 20:56             ` Lawrence Velázquez
2021-04-10 21:22               ` Bart Schaefer
2021-04-13 11:46                 ` Daniel Shahaf
2021-04-13 21:33                   ` Bart Schaefer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAEtFKssJVz9mr3vA_vY77m2bsrXWBKHz1qgv83jzeY0jLXzPjQ@mail.gmail.com \
    --to=husseydevin@gmail.com \
    --cc=schaefer@brasslantern.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).