[PATCH] Ignore EACCES when doing non-pure globbing

[PATCH] Ignore EACCES when doing non-pure globbing

[Devin Hussey]

Even if we can't access a parent folder, there is still a chance we can
access a subdirectory.

This is notoriously the case on Android, where apps can access their own
subfolders in /data, but not /data itself.

This was causing many issues with Termux, the terminal emulator which
runs from /data on Android, as many scripts will try to glob relative to
the home directory, and it would error when it tried to opendir() on /data.

Over-recursion does not seem to be an issue, as EACCES seems to have
lower priority than ENOENT or ENOTDIR, at least on Linux.

See:
 - https://github.com/sorin-ionescu/prezto/issues/1560
 - https://github.com/termux/termux-packages/issues/1894

Signed-off-by: Devin Hussey <husseydevin@gmail.com>
---
 Src/glob.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Src/glob.c b/Src/glob.c
index bee890caf..7b9414c6f 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -585,9 +585,11 @@ scanner(Complist q, int shortcircuit)
 	char *subdirs = NULL;
 	int subdirlen = 0;

-	if (lock == NULL)
+	/* Ignore EACCES. We may be in a jail like Android's /data where we can
+	 * only access specific subfolders. */
+	if (lock == NULL && errno != EACCES)
 	    return;
-	while ((fn = zreaddir(lock, 1)) && !errflag) {
+	while (lock != NULL && (fn = zreaddir(lock, 1)) && !errflag) {
 	    /* prefix and suffix are zle trickery */
 	    if (!dirs && !colonmod &&
 		((glob_pre && !strpfx(glob_pre, fn))
@@ -673,7 +675,8 @@ scanner(Complist q, int shortcircuit)
 		}
 	    }
 	}
-	closedir(lock);
+	if (lock != NULL)
+	    closedir(lock);
 	if (subdirs) {
 	    int oppos = pathpos;

-- 
2.30.0

Re: [PATCH] Ignore EACCES when doing non-pure globbing

[Devin Hussey]

This hasn't been fully tested, but Prezto is seemingly working on
Termux with this patch.

You can simulate this condition like so (as Michael mentioned):

% mkdir inaccessible; cd inaccessible
% mkdir a; cd a
% chmod 000 .. # make inaccessible inaccessible
% touch a b c; echo *
a b c
% echo $PWD/*
zsh: no matches found: /tmp/inaccessible/a/*
# or "/tmp/inaccessible/a/*"

Re: [PATCH] Ignore EACCES when doing non-pure globbing

[Lawrence Velázquez]

> On Jan 12, 2021, at 5:42 PM, Devin Hussey <husseydevin@gmail.com> wrote:
> 
> Even if we can't access a parent folder, there is still a chance we can
> access a subdirectory.

I might be mistaken (entirely possible!), but this behavior seems
to violate POSIX volume 3 chapter 2 section 2.13.3, which states
in part that

	Specified patterns shall be matched against existing filenames
	and pathnames, as appropriate. Each component that contains a
	pattern character shall require read permission in the directory
	containing that component. Any component, except the last, that
	does not contain a pattern character shall require search permission.

(https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_13_03)

vq

Re: [PATCH] Ignore EACCES when doing non-pure globbing

[Devin Hussey]

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

We are already violating POSIX with our globbing:

"If the pattern does not match any existing filenames or pathnames, the
pattern string shall be left unchanged."

Therefore, this:

zsh:1: no matches found: /tmp/inaccessible/a/*

is wrong, it should just print this:

/tmp/inaccessible/a/*

Also, at least with the implementation on my device, the glob() function
works fine as long as the folder has execute permission. (The sample was
wrong, it should be 111, not 000)

On Tue, Jan 12, 2021, 6:47 PM Lawrence Velázquez <vq@larryv.me> wrote:

> > On Jan 12, 2021, at 5:42 PM, Devin Hussey <husseydevin@gmail.com> wrote:
> >
> > Even if we can't access a parent folder, there is still a chance we can
> > access a subdirectory.
>
> I might be mistaken (entirely possible!), but this behavior seems
> to violate POSIX volume 3 chapter 2 section 2.13.3, which states
> in part that
>
>         Specified patterns shall be matched against existing filenames
>         and pathnames, as appropriate. Each component that contains a
>         pattern character shall require read permission in the directory
>         containing that component. Any component, except the last, that
>         does not contain a pattern character shall require search
> permission.
>
> (
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_13_03
> )
>
> vq

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

Re: [PATCH] Ignore EACCES when doing non-pure globbing

[Devin Hussey]

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

Actually, scratch that, I can confirm that Android has their permissions
set to 711 on the /data folder. Otherwise we wouldn't be able to cd to
them. And the 1 bit *is* the searchable bit (not the executable bit) for
directories (TIL), so /data is searchable.

So, both the pre-patch and post-patch behaviors are noncompliant. The first
is too strict and this is too lenient (although I prefer more lenient than
strict). Back to the drawing board, I guess.

On Tue, Jan 12, 2021, 7:53 PM Devin Hussey <husseydevin@gmail.com> wrote:

> We are already violating POSIX with our globbing:
>
> "If the pattern does not match any existing filenames or pathnames, the
> pattern string shall be left unchanged."
>
> Therefore, this:
>
> zsh:1: no matches found: /tmp/inaccessible/a/*
>
> is wrong, it should just print this:
>
> /tmp/inaccessible/a/*
>
> Also, at least with the implementation on my device, the glob() function
> works fine as long as the folder has execute permission. (The sample was
> wrong, it should be 111, not 000)
>
> On Tue, Jan 12, 2021, 6:47 PM Lawrence Velázquez <vq@larryv.me> wrote:
>
>> > On Jan 12, 2021, at 5:42 PM, Devin Hussey <husseydevin@gmail.com>
>> wrote:
>> >
>> > Even if we can't access a parent folder, there is still a chance we can
>> > access a subdirectory.
>>
>> I might be mistaken (entirely possible!), but this behavior seems
>> to violate POSIX volume 3 chapter 2 section 2.13.3, which states
>> in part that
>>
>>         Specified patterns shall be matched against existing filenames
>>         and pathnames, as appropriate. Each component that contains a
>>         pattern character shall require read permission in the directory
>>         containing that component. Any component, except the last, that
>>         does not contain a pattern character shall require search
>> permission.
>>
>> (
>> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_13_03
>> )
>>
>> vq
>
>

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

Re: [PATCH] Ignore EACCES when doing non-pure globbing

[Bart Schaefer]

On Tue, Jan 12, 2021 at 5:12 PM Devin Hussey <husseydevin@gmail.com> wrote:
>
> So, both the pre-patch and post-patch behaviors are noncompliant. The first is too strict and this is too lenient (although I prefer more lenient than strict). Back to the drawing board, I guess.

There are options (shglob, shfileexpansion, no_nomatch, etc.)
controlling whether the globbing behavior does or does not conform to
POSIX.  It's not a problem that "no matches found" is printed when the
shell is not in POSIX mode.  It would be a problem if "too many"
matches were found when the shell IS in POSIX mode.  So it may be that
any patch here has to take one or more of those setopts into account.

That said, the POSIX requirement is only that any "component that
contains a pattern character" must have read permission on the
directory containing it.  A "component" here is what appears between
two "/" (or after the rightmost one).  So I don't think there was a
problem with that requirement.

Re: [PATCH] Ignore EACCES when doing non-pure globbing

[Mikael Magnusson]

On 1/13/21, Devin Hussey <husseydevin@gmail.com> wrote:
> We are already violating POSIX with our globbing:
>
> "If the pattern does not match any existing filenames or pathnames, the
> pattern string shall be left unchanged."
>
> Therefore, this:
>
> zsh:1: no matches found: /tmp/inaccessible/a/*
>
> is wrong, it should just print this:
>
> /tmp/inaccessible/a/*

If you want the broken POSIX behavior you can say 'setopt nonomatch'.

> Also, at least with the implementation on my device, the glob() function
> works fine as long as the folder has execute permission. (The sample was
> wrong, it should be 111, not 000)

This makes quite a big difference indeed. Have you considered just not
unsetting caseglob?
Also of note (maybe) (without unsetting caseglob obviously) (and
setting extendedglob),
% echo $PWD/(#i)a*
/tmp/inaccessible/a/A /tmp/inaccessible/a/a
% echo (#i)$PWD/a*
zsh: no matches found: (#i)/tmp/inaccessible/a/a*

Eg, as long as you don't ask for case insensitiveness for the
unreadable segments, you're fine.

-- 
Mikael Magnusson

Re: [PATCH] Ignore EACCES when doing non-pure globbing

[Devin Hussey]

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

Actually replied to the mailing list this time...

>Have you considered just not unsetting caseglob?

Brilliant solution.
"Doctor, it hurts when I do this."
"Then don't do that."

As for the compliant version, here is a much more "polite" version which
also avoids the awkward checks if lock != NULL (at the cost of reformatting
O.o).

/* Check for search permissions. If we don't have them, get out. */
if (access(fn, X_OK) != 0) {
    return;
}
/* Check for read permissions. If we have them, try to open the directory.
*/
if (access(fn, R_OK) == 0) {
    DIR *lock = diropen(fn);
    if (lock == NULL)
        return;

    while (...) { ... }
    closedir(lock);
}
if (subdirs) { ... }


% chmod 111 ..
% echo $PWD/*
/tmp/inaccessible/a/a /tmp/inaccessible/a/b /tmp/inaccessible/a/c
% chmod 000 ..
% echo $PWD/*
zsh: no matches found: /tmp/inaccessible/a/*

This matches the POSIX behavior of requiring parent directories to be
searchable, and is a little more respectful to filesystem permissions.

On Tue, Jan 12, 2021, 8:58 PM Devin Hussey <husseydevin@gmail.com> wrote:

> >Have you considered just not unsetting caseglob?
>
> Brilliant solution.
> "Doctor, it hurts when I do this."
> "Then don't do that."
>
> As for the compliant version, here is a much more "polite" version.
>
> This matches the behavior of the POSIX globs: with a non searchable
> directory, it returns no match, but doesn't require read access for parent
> directories.
>
> DIR *lock = NULL;
>
> /* Check for search permissions. If we don't have them, get out. */
> if (access(fn, X_OK) != 0) {
>     return;
> }
> /* Check for read permissions. If we have them, try to open the directory.
> */
> if (access(fn, R_OK) == 0) {
>     lock = diropen(fn);
>     /* Error */
>     if (lock == NULL)
>         return;
> }
> while (lock != NULL &&...
>
> % chmod 111 ..
> % echo $PWD/*
> /tmp/inaccessible/a/a /tmp/inaccessible/a/b /tmp/inaccessible/a/c
> % chmod 000 ..
> % echo $PWD/*
> zsh: no matches found: /tmp/inaccessible/a/*
>

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

Re: [PATCH] Ignore EACCES when doing non-pure globbing

[Devin Hussey]

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

I am abandoning this hack in favor of the next patch.

On Tue, Jan 12, 2021, 9:14 PM Devin Hussey <husseydevin@gmail.com> wrote:

> Actually replied to the mailing list this time...
>
> >Have you considered just not unsetting caseglob?
>
> Brilliant solution.
> "Doctor, it hurts when I do this."
> "Then don't do that."
>
> As for the compliant version, here is a much more "polite" version which
> also avoids the awkward checks if lock != NULL (at the cost of reformatting
> O.o).
>
> /* Check for search permissions. If we don't have them, get out. */
> if (access(fn, X_OK) != 0) {
>     return;
> }
> /* Check for read permissions. If we have them, try to open the directory.
> */
> if (access(fn, R_OK) == 0) {
>     DIR *lock = diropen(fn);
>     if (lock == NULL)
>         return;
>
>     while (...) { ... }
>     closedir(lock);
> }
> if (subdirs) { ... }
>
>
> % chmod 111 ..
> % echo $PWD/*
> /tmp/inaccessible/a/a /tmp/inaccessible/a/b /tmp/inaccessible/a/c
> % chmod 000 ..
> % echo $PWD/*
> zsh: no matches found: /tmp/inaccessible/a/*
>
> This matches the POSIX behavior of requiring parent directories to be
> searchable, and is a little more respectful to filesystem permissions.
>
> On Tue, Jan 12, 2021, 8:58 PM Devin Hussey <husseydevin@gmail.com> wrote:
>
>> >Have you considered just not unsetting caseglob?
>>
>> Brilliant solution.
>> "Doctor, it hurts when I do this."
>> "Then don't do that."
>>
>> As for the compliant version, here is a much more "polite" version.
>>
>> This matches the behavior of the POSIX globs: with a non searchable
>> directory, it returns no match, but doesn't require read access for parent
>> directories.
>>
>> DIR *lock = NULL;
>>
>> /* Check for search permissions. If we don't have them, get out. */
>> if (access(fn, X_OK) != 0) {
>>     return;
>> }
>> /* Check for read permissions. If we have them, try to open the
>> directory. */
>> if (access(fn, R_OK) == 0) {
>>     lock = diropen(fn);
>>     /* Error */
>>     if (lock == NULL)
>>         return;
>> }
>> while (lock != NULL &&...
>>
>> % chmod 111 ..
>> % echo $PWD/*
>> /tmp/inaccessible/a/a /tmp/inaccessible/a/b /tmp/inaccessible/a/c
>> % chmod 000 ..
>> % echo $PWD/*
>> zsh: no matches found: /tmp/inaccessible/a/*
>>
>

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