From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/14504 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] In glob(), do not require that the target of a symlink exists. Date: Mon, 5 Aug 2019 22:03:20 -0400 Message-ID: <20190806020320.GR9017@brightrain.aerifal.cx> References: Reply-To: musl@lists.openwall.com Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="238056"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-14520-gllmg-musl=m.gmane.org@lists.openwall.com Tue Aug 06 04:03:39 2019 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.89) (envelope-from ) id 1huop9-000zs6-BD for gllmg-musl@m.gmane.org; Tue, 06 Aug 2019 04:03:39 +0200 Original-Received: (qmail 32520 invoked by uid 550); 6 Aug 2019 02:03:37 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 32448 invoked from network); 6 Aug 2019 02:03:32 -0000 Content-Disposition: inline In-Reply-To: Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:14504 Archived-At: On Mon, Jul 22, 2019 at 08:37:38PM -0400, James Y Knight wrote: > Previously, when given a trailing path component with no > metacharacters, glob would call stat to check if the provided filename > existed, which would fail on a broken symlink. When expanding a > pattern however, glob would trust the list of files returned by > readdir, and thus would return broken symlinks. > > Now, be consistent and allow broken symlinks in both cases, by using > lstat to determine file existence. > > If GLOB_MARK is specified, stat must still be used to determine > whether a given name refers to a directory or file. But if that fails, > a symlink is simply considered to be a file for marking purposes. > From 7ef0fabf173f4d29461b01ccfb05c1f19977ba37 Mon Sep 17 00:00:00 2001 > From: James Y Knight > Date: Thu, 11 Jul 2019 16:55:17 -0400 > Subject: [PATCH] In glob(), do not require that the target of a symlink > exists. > > Previously, when given a trailing path component with no > metacharacters, glob would call stat to check if the provided filename > existed, which would fail on a broken symlink. When expanding a > pattern however, glob would trust the list of files returned by > readdir, and thus would return broken symlinks. > > Now, be consistent and allow broken symlinks in both cases, by using > lstat to determine file existence. > > If GLOB_MARK is specified, stat must still be used to determine > whether a given name refers to a directory or file. But if that fails, > a symlink is simply considered to be a file for marking purposes. > --- > src/regex/glob.c | 35 +++++++++++++++++++++++++---------- > 1 file changed, 25 insertions(+), 10 deletions(-) > > diff --git a/src/regex/glob.c b/src/regex/glob.c > index aa1c6a44..01b178dc 100644 > --- a/src/regex/glob.c > +++ b/src/regex/glob.c > @@ -88,18 +88,33 @@ static int do_glob(char *buf, size_t pos, int type, char *pat, int flags, int (* > } > buf[pos] = 0; > if (!*pat) { > - /* If we consumed any components above, or if GLOB_MARK is > - * requested and we don't yet know if the match is a dir, > - * we must call stat to confirm the file exists and/or > - * determine its type. */ > + /* If we're in at the end of the pattern, check if the file > + * exists. And, if GLOB_MARK is requested, determine if it's a > + * directory. */ This is one of the comment changes I was confused about; it seems less informative. The point of the original comment was not describing the controlling expression just above it, but the conditions on not being able to use the caller-passed type -- having consumed any new literal components from the pattern above is what precludes it. > struct stat st; > - if ((flags & GLOB_MARK) && type==DT_LNK) type = 0; > - if (!type && stat(buf, &st)) { > - if (errno!=ENOENT && (errfunc(buf, errno) || (flags & GLOB_ERR))) > - return GLOB_ABORTED; > - return 0; > + if (flags & GLOB_MARK) { > + /* We need to know if the name refers to a directory > + * (after resolving symlinks). > + * > + * However, broken symlinks should be considered to be a > + * file, rather than non-existent or an error, so if > + * stat fails, we just don't modify type. (And lstat > + * will be called below if required.) > + */ > + if ((!type || type==DT_LNK) && stat(buf, &st) == 0) { !stat > + if(S_ISDIR(st.st_mode)) type = DT_DIR; > + else type = DT_REG; > + } > + } > + if (!type) { > + /* If we're don't already know that the file exists, > + * confirm its presence. */ > + if (lstat(buf, &st)) { This could be "!type && lstat(..." to avoid the excessively long lines below and to better mimic the old structure so that stylistic change doesn't obscure reading of functional change. Now that I look at it, the same applies to the above too. > + if (errno!=ENOENT && (errfunc(buf, errno) || (flags & GLOB_ERR))) > + return GLOB_ABORTED; > + return 0; > + } > } > - if (!type && S_ISDIR(st.st_mode)) type = DT_DIR; > if (append(tail, buf, pos, (flags & GLOB_MARK) && type==DT_DIR)) > return GLOB_NOSPACE; > return 0; > -- > 2.22.0.657.g960e92d24f-goog > Otherwise I think this looks ok. Rich