getusershell: rewrite from scratch [BZ #18660]
Commit Message
The current getusershell implementation is ancient, buggy, and overly
complicated. Rewrite it from scratch and exercise all the demons.
This does change the API a bit: the old one basically did:
- skip all leading bytes until a '#' or '/' is found
- if end of line, skip the line
- if byte is '#', skip the line
- if byte is '/' and is the end of file, skip the line
- if byte is '/', scan all bytes until [[:space:]#\0] is hit, set that
byte to NUL, and return the line
This is pretty idiosyncratic behavior. Some examples:
" /foo bar" -> "/foo"
" /fat#cat" -> "/fat"
"space/cow" -> "/cow"
"/" -> "/" (unless this is the last byte in the file!)
The new one is much simpler: if the first byte is a '/', then return
the line as-is. Everything else is ignored.
Comparing the API to other projects:
- gnulib returns any line that does not start with '#'
- FreeBSD behaves same as glibc before this, except it allows / at the end
- OpenBSD behaves the same as FreeBSD
- Darwin behaves the same as FreeBSD
- shadow's chsh ignores lines that start with '#' but allows all others
- util-linux's chsh ignores lines that start with '#', is just "", or if
the last byte of the file is '/', but allows all others
Comparing it to documentation of getusershell(3):
- glibc doesn't document this function
- Linux man-pages just says "The line should contain the pathname of a
valid user shell."
- gnulib says "Return names of valid user shells."
- FreeBSD man says "... returns a pointer to a valid user shell as defined
by the system manager in the shells database as described in shells(5)."
- OpenBSD man says "... returns a pointer to a legal user shell as defined
by the system manager in the file /etc/shells."
- Darwin says the same as OpenBSD
And to the shells(5) documentation:
- Linux man-pages says "... a text file which contains the full pathnames
of valid login shells."
- FreeBSD says "The shells file contains a list of the shells on the
system. For each shell a single line should be present, consisting of
the shell's path, relative to root. A hash mark (``#'') indicates the
beginning of a comment; subsequent characters up to the end of the line
are not interpreted by the routines which search the file. Blank lines
are also ignored."
- OpenBSD says pretty much the same thing as FreeBSD
- Darwin says the same as OpenBSD
And to the chsh(1) documentation:
- shadow says "... login shell is that the command name must be listed in
/etc/shells, ..." and "/etc/shells: List of valid login shells"
- util-linux says "chsh will accept the full pathname of any executable
file on the system. However, it will issue a warning if the shell is
not listed in the /etc/shells file."
I think the new glibc behavior matches the intentions of these APIs while
producing small code. Namely, getusershell is supposed to return "valid"
shells from /etc/shells, and people define that as absolute paths to an
interactive program. Relative paths are dangerous and undesirable, as are
random lines w/out any paths at all. Arbitrarily splitting lines on space
and hash marks is also dangerous/surprising.
2015-10-19 Mike Frysinger <vapier@gentoo.org>
[BZ #18660]
* misc/Makefile (tests): Add tst-getusershell and tst-endusershell.
* misc/getusershell.c: Rewrite from scratch.
* misc/tst-getusershell.c: New test.
* misc/tst-endusershell.c: Likewise.
---
NEWS | 4 +
misc/Makefile | 3 +-
misc/getusershell.c | 213 +++++++++++++++++++++---------------------------
misc/tst-endusershell.c | 64 +++++++++++++++
misc/tst-getusershell.c | 178 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 339 insertions(+), 123 deletions(-)
create mode 100644 misc/tst-endusershell.c
create mode 100644 misc/tst-getusershell.c
Comments
Mike Frysinger <vapier@gentoo.org> writes:
> - FreeBSD says "The shells file contains a list of the shells on the
> system. For each shell a single line should be present, consisting of
> the shell's path, relative to root. A hash mark (``#'') indicates the
> beginning of a comment; subsequent characters up to the end of the line
> are not interpreted by the routines which search the file. Blank lines
> are also ignored."
This is the ultimate documentation, as the interface was invented by
BSD. Why deviate from it?
Andreas.
On Wed, 21 Oct 2015, Mike Frysinger wrote:
> +/* Copyright (C) 2015 Free Software Foundation, Inc.
The first line of any new file should be a descriptive comment, before the
copyright notice.
On 21 Oct 2015 09:47, Andreas Schwab wrote:
> Mike Frysinger <vapier@gentoo.org> writes:
> > - FreeBSD says "The shells file contains a list of the shells on the
> > system. For each shell a single line should be present, consisting of
> > the shell's path, relative to root. A hash mark (``#'') indicates the
> > beginning of a comment; subsequent characters up to the end of the line
> > are not interpreted by the routines which search the file. Blank lines
> > are also ignored."
>
> This is the ultimate documentation, as the interface was invented by
> BSD. Why deviate from it?
i disagree. as i've shown, there is deviation between systems already,
which means admins cannot rely on the behavior being the same everywhere.
hence i turn to the intention of the API and how a reasonable admin would
configure the system, and i think this new version follows that.
note that the API/ABI for tools is unchanged here -- programs all continue
to run and behave the same. it's changing how the contents of the shells
file is parsed and what entries are returned.
-mike
Mike,
I like this implementation as it really an improvement to the status quo (IMHO). Also, the test cases are a great benefit as well :)
Best regards
Tolga Dalman
On 10/21/2015 07:39 AM, Mike Frysinger wrote:
> The current getusershell implementation is ancient, buggy, and overly
> complicated. Rewrite it from scratch and exercise all the demons.
>
> This does change the API a bit: the old one basically did:
> - skip all leading bytes until a '#' or '/' is found
> - if end of line, skip the line
> - if byte is '#', skip the line
> - if byte is '/' and is the end of file, skip the line
> - if byte is '/', scan all bytes until [[:space:]#\0] is hit, set that
> byte to NUL, and return the line
> This is pretty idiosyncratic behavior. Some examples:
> " /foo bar" -> "/foo"
> " /fat#cat" -> "/fat"
> "space/cow" -> "/cow"
> "/" -> "/" (unless this is the last byte in the file!)
>
> The new one is much simpler: if the first byte is a '/', then return
> the line as-is. Everything else is ignored.
>
> Comparing the API to other projects:
> - gnulib returns any line that does not start with '#'
> - FreeBSD behaves same as glibc before this, except it allows / at the end
> - OpenBSD behaves the same as FreeBSD
> - Darwin behaves the same as FreeBSD
> - shadow's chsh ignores lines that start with '#' but allows all others
> - util-linux's chsh ignores lines that start with '#', is just "", or if
> the last byte of the file is '/', but allows all others
>
> Comparing it to documentation of getusershell(3):
> - glibc doesn't document this function
> - Linux man-pages just says "The line should contain the pathname of a
> valid user shell."
> - gnulib says "Return names of valid user shells."
> - FreeBSD man says "... returns a pointer to a valid user shell as defined
> by the system manager in the shells database as described in shells(5)."
> - OpenBSD man says "... returns a pointer to a legal user shell as defined
> by the system manager in the file /etc/shells."
> - Darwin says the same as OpenBSD
>
> And to the shells(5) documentation:
> - Linux man-pages says "... a text file which contains the full pathnames
> of valid login shells."
> - FreeBSD says "The shells file contains a list of the shells on the
> system. For each shell a single line should be present, consisting of
> the shell's path, relative to root. A hash mark (``#'') indicates the
> beginning of a comment; subsequent characters up to the end of the line
> are not interpreted by the routines which search the file. Blank lines
> are also ignored."
> - OpenBSD says pretty much the same thing as FreeBSD
> - Darwin says the same as OpenBSD
>
> And to the chsh(1) documentation:
> - shadow says "... login shell is that the command name must be listed in
> /etc/shells, ..." and "/etc/shells: List of valid login shells"
> - util-linux says "chsh will accept the full pathname of any executable
> file on the system. However, it will issue a warning if the shell is
> not listed in the /etc/shells file."
>
> I think the new glibc behavior matches the intentions of these APIs while
> producing small code. Namely, getusershell is supposed to return "valid"
> shells from /etc/shells, and people define that as absolute paths to an
> interactive program. Relative paths are dangerous and undesirable, as are
> random lines w/out any paths at all. Arbitrarily splitting lines on space
> and hash marks is also dangerous/surprising.
>
> 2015-10-19 Mike Frysinger <vapier@gentoo.org>
>
> [BZ #18660]
> * misc/Makefile (tests): Add tst-getusershell and tst-endusershell.
> * misc/getusershell.c: Rewrite from scratch.
> * misc/tst-getusershell.c: New test.
> * misc/tst-endusershell.c: Likewise.
> ---
> NEWS | 4 +
> misc/Makefile | 3 +-
> misc/getusershell.c | 213 +++++++++++++++++++++---------------------------
> misc/tst-endusershell.c | 64 +++++++++++++++
> misc/tst-getusershell.c | 178 ++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 339 insertions(+), 123 deletions(-)
> create mode 100644 misc/tst-endusershell.c
> create mode 100644 misc/tst-getusershell.c
>
> diff --git a/NEWS b/NEWS
> index 1313ab2..4ba05e2 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -22,6 +22,10 @@ Version 2.23
> 19049, 19050, 19059, 19071, 19074, 19076, 19077, 19078, 19079, 19085,
> 19086, 19088, 19094, 19095, 19124, 19125, 19129, 19134, 19137.
>
> +* The getusershell function now returns entries from /etc/shells only when
> + the line starts with a '/', and no longer chops lines based on '#' or white
> + space.
> +
> * There is now a --disable-timezone-tools configure option for disabling the
> building and installing of the timezone related utilities (zic, zdump, and
> tzselect). This is useful for people who build the timezone data and code
> diff --git a/misc/Makefile b/misc/Makefile
> index 2f5edf6..3fc44f1 100644
> --- a/misc/Makefile
> +++ b/misc/Makefile
> @@ -77,7 +77,8 @@ gpl2lgpl := error.c error.h
>
> tests := tst-dirname tst-tsearch tst-fdset tst-efgcvt tst-mntent tst-hsearch \
> tst-error1 tst-pselect tst-insremque tst-mntent2 bug-hsearch1 \
> - tst-mntent-blank-corrupt tst-mntent-blank-passno
> + tst-mntent-blank-corrupt tst-mntent-blank-passno tst-getusershell \
> + tst-endusershell
> ifeq ($(run-built-tests),yes)
> tests-special += $(objpfx)tst-error1-mem.out
> endif
> diff --git a/misc/getusershell.c b/misc/getusershell.c
> index fc2c43b..3d3f221 100644
> --- a/misc/getusershell.c
> +++ b/misc/getusershell.c
> @@ -1,143 +1,112 @@
> -/*
> - * Copyright (c) 1985, 1993
> - * The Regents of the University of California. All rights reserved.
> - *
> - * Redistribution and use in source and binary forms, with or without
> - * modification, are permitted provided that the following conditions
> - * are met:
> - * 1. Redistributions of source code must retain the above copyright
> - * notice, this list of conditions and the following disclaimer.
> - * 2. Redistributions in binary form must reproduce the above copyright
> - * notice, this list of conditions and the following disclaimer in the
> - * documentation and/or other materials provided with the distribution.
> - * 4. Neither the name of the University nor the names of its contributors
> - * may be used to endorse or promote products derived from this software
> - * without specific prior written permission.
> - *
> - * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
> - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> - * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
> - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> - * SUCH DAMAGE.
> - */
> -
> -#if defined(LIBC_SCCS) && !defined(lint)
> -static char sccsid[] = "@(#)getusershell.c 8.1 (Berkeley) 6/4/93";
> -#endif /* LIBC_SCCS and not lint */
> -
> -#include <sys/param.h>
> -#include <sys/file.h>
> -#include <sys/stat.h>
> +/* Copyright (C) 2015 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <assert.h>
> +#include <paths.h>
> #include <stdio.h>
> -#include <stdio_ext.h>
> -#include <ctype.h>
> #include <stdlib.h>
> +#include <string.h>
> #include <unistd.h>
> -#include <paths.h>
>
> -/*
> - * Local shells should NOT be added here. They should be added in
> - * /etc/shells.
> - */
> -
> -/* NB: we do not initialize okshells here. The initialization needs
> - relocations. These interfaces are used so rarely that this is not
> - justified. Instead explicitly initialize the array when it is
> - used. */
> -#if 0
> -static const char *const okshells[] = { _PATH_BSHELL, _PATH_CSHELL, NULL };
> -#else
> -static const char *okshells[3];
> +/* Used by tests to point to custom paths. */
> +#ifndef _LOCAL_PATH_SHELLS
> +# define _LOCAL_PATH_SHELLS _PATH_SHELLS
> #endif
> -static char **curshell, **shells, *strings;
> -static char **initshells (void) __THROW;
>
> -/*
> - * Get a list of shells from _PATH_SHELLS, if it exists.
> - */
> +/* These functions are not thread safe (by design), so globals are OK. */
> +static FILE *shellfp;
> +static int okshell_idx;
> +static char *shellbuf;
> +static size_t buf_len;
> +
> char *
> getusershell (void)
> {
> - char *ret;
> -
> - if (curshell == NULL)
> - curshell = initshells();
> - ret = *curshell;
> - if (ret != NULL)
> - curshell++;
> - return (ret);
> + /* Handle the builtin case first.
> +
> + NB: we do not use a global array here as the initialization needs
> + relocations. These interfaces are used so rarely that this is not
> + justified. Instead open code it with a switch statement. */
> + switch (okshell_idx)
> + {
> + case 0:
> + if (shellfp == NULL)
> + {
> + setusershell ();
> + return getusershell ();
> + }
> + break;
> + case 1:
> + okshell_idx++;
> + return (char *) _PATH_BSHELL;
> + case 2:
> + okshell_idx++;
> + return (char *) _PATH_CSHELL;
> + case 3:
> + return NULL;
> + }
> +
> + /* Walk the /etc/shells file. */
> + while (1)
> + {
> + ssize_t len;
> + char *p;
> +
> + len = getline (&shellbuf, &buf_len, shellfp);
> + if (len <= 0)
> + break;
> +
> + /* Chop the trailing newline. */
> + p = shellbuf + len - 1;
> + if (*p == '\n')
> + *p = '\0';
> +
> + /* Only accept valid shells (which we define as starting with a '/'). */
> + if (shellbuf[0] == '/')
> + return shellbuf;
> + }
> +
> + /* No more valid shells. */
> + return NULL;
> }
>
> +hidden_proto (endusershell);
> void
> endusershell (void)
> {
> -
> - free(shells);
> - shells = NULL;
> - free(strings);
> - strings = NULL;
> - curshell = NULL;
> + okshell_idx = 0;
> + if (shellfp)
> + {
> + fclose (shellfp);
> + shellfp = NULL;
> + free (shellbuf);
> + shellbuf = NULL;
> + }
> }
> +hidden_def (endusershell);
>
> void
> setusershell (void)
> {
> + /* We could rewind shellfp here, but we get smaller code this way.
> + Keep in mind this is not a performance sensitive API. */
> + endusershell ();
>
> - curshell = initshells();
> -}
> -
> -static char **
> -initshells (void)
> -{
> - char **sp, *cp;
> - FILE *fp;
> - struct stat64 statb;
> - size_t flen;
> -
> - free(shells);
> - shells = NULL;
> - free(strings);
> - strings = NULL;
> - if ((fp = fopen(_PATH_SHELLS, "rce")) == NULL)
> - goto init_okshells_noclose;
> - if (fstat64(fileno(fp), &statb) == -1) {
> - init_okshells:
> - (void)fclose(fp);
> - init_okshells_noclose:
> - okshells[0] = _PATH_BSHELL;
> - okshells[1] = _PATH_CSHELL;
> - return (char **) okshells;
> - }
> - if (statb.st_size > ~(size_t)0 / sizeof (char *) * 3)
> - goto init_okshells;
> - flen = statb.st_size + 3;
> - if ((strings = malloc(flen)) == NULL)
> - goto init_okshells;
> - shells = malloc(statb.st_size / 3 * sizeof (char *));
> - if (shells == NULL) {
> - free(strings);
> - strings = NULL;
> - goto init_okshells;
> - }
> - sp = shells;
> - cp = strings;
> - while (fgets_unlocked(cp, flen - (cp - strings), fp) != NULL) {
> - while (*cp != '#' && *cp != '/' && *cp != '\0')
> - cp++;
> - if (*cp == '#' || *cp == '\0' || cp[1] == '\0')
> - continue;
> - *sp++ = cp;
> - while (!isspace(*cp) && *cp != '#' && *cp != '\0')
> - cp++;
> - *cp++ = '\0';
> - }
> - *sp = NULL;
> - (void)fclose(fp);
> - return (shells);
> + shellfp = fopen (_LOCAL_PATH_SHELLS, "rce");
> + if (shellfp == NULL)
> + okshell_idx = 1;
> }
> diff --git a/misc/tst-endusershell.c b/misc/tst-endusershell.c
> new file mode 100644
> index 0000000..95c2143
> --- /dev/null
> +++ b/misc/tst-endusershell.c
> @@ -0,0 +1,64 @@
> +/* Make sure calling endusershell first doesn't crash.
> +
> + Copyright (C) 2015 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +static int do_test (void);
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> +
> +static int
> +do_test (void)
> +{
> + /* This should not crash or corrupt. */
> + endusershell ();
> + endusershell ();
> + endusershell ();
> +
> + /* Neither should this. */
> + setusershell ();
> + setusershell ();
> + setusershell ();
> + setusershell ();
> + setusershell ();
> +
> + /* Shouldn't be a problem, but we can't really verify the return since
> + it depends on /etc/shells in the installed system. */
> + getusershell ();
> + getusershell ();
> + getusershell ();
> + getusershell ();
> + getusershell ();
> + getusershell ();
> +
> + /* Test resetting behavior. */
> + setusershell ();
> + getusershell ();
> + setusershell ();
> + getusershell ();
> +
> + /* Test circular behavior. */
> + setusershell ();
> + getusershell ();
> + endusershell ();
> + setusershell ();
> + getusershell ();
> + endusershell ();
> +
> + return 0;
> +}
> diff --git a/misc/tst-getusershell.c b/misc/tst-getusershell.c
> new file mode 100644
> index 0000000..27fc4f8
> --- /dev/null
> +++ b/misc/tst-getusershell.c
> @@ -0,0 +1,178 @@
> +/* Verify behavior of getusershell w/various inputs.
> +
> + Copyright (C) 2015 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +/* Set the functions to read from our custom file so we can control the
> + exact content for the tests. */
> +static char *path_shells;
> +#define _LOCAL_PATH_SHELLS path_shells
> +#include "getusershell.c"
> +
> +#include <assert.h>
> +
> +static int do_test (void);
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> +
> +static int path_shells_fd;
> +
> +static void
> +set_content (const char *content)
> +{
> + size_t len = strlen (content);
> +
> + assert (lseek (path_shells_fd, 0, SEEK_SET) == 0);
> + assert (ftruncate (path_shells_fd, 0) == 0);
> + assert (write (path_shells_fd, content, len) == len);
> +
> + /* Reset the shell reading state since we rewrote the file. */
> + endusershell ();
> +}
> +
> +#define dprintf(fmt, args...) printf ("%s: " fmt, __func__, ## args)
> +
> +/* Verify behavior of getusershell w/out any /etc/shells file. */
> +static int
> +check_default (void)
> +{
> + char bad_shells[] = "";
> + char *old_shells = path_shells;
> + const char *shell;
> + int ret = 1, i;
> +
> + /* This will cause fopen to fail. */
> + path_shells = bad_shells;
> +
> + shell = getusershell ();
> + if (shell == NULL || strcmp (shell, "/bin/sh") != 0)
> + {
> + dprintf ("first shell must be /bin/sh, not '%s'\n", shell);
> + goto done;
> + }
> +
> + shell = getusershell ();
> + if (shell == NULL || strcmp (shell, "/bin/csh") != 0)
> + {
> + dprintf ("second shell must be /bin/csh, not '%s'\n", shell);
> + goto done;
> + }
> +
> + /* Call it a few times to make sure it doesn't wrap or anything. */
> + for (i = 0; i < 10; ++i)
> + {
> + shell = getusershell ();
> + if (shell != NULL)
> + {
> + dprintf ("third shell must be NULL, not '%s'\n", shell);
> + goto done;
> + }
> + }
> +
> + ret = 0;
> + done:
> + path_shells = old_shells;
> + return ret;
> +}
> +
> +/* An empty file should return NULL forever. */
> +static int
> +check_empty (void)
> +{
> + int i;
> + char *shell;
> +
> + set_content ("");
> +
> + for (i = 0; i < 10; ++i)
> + {
> + shell = getusershell ();
> + if (shell != NULL)
> + {
> + dprintf ("empty file should return NULL, not '%s'\n", shell);
> + return 1;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/* Verify we handle comments and blank lines correctly. */
> +static int
> +check_comments_blank_lines (void)
> +{
> + int i;
> + char *shell;
> +
> + set_content (
> + /* A mix of comments and blanks. */
> + "# A comment\n"
> + " # A comment w/leading spaces\n"
> + "\t\t# A comment w/leading tabs\n"
> + /* Now just blank lines. */
> + "\n"
> + " \n"
> + "\t \t\n"
> + );
> +
> + for (i = 0; i < 10; ++i)
> + {
> + shell = getusershell ();
> + if (shell != NULL)
> + {
> + dprintf ("comment/blank file should return NULL, not '%s'\n", shell);
> + return 1;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/* Verify "valid" shells which is defined by starting with a /. */
> +static int
> +check_valid_shells (void)
> +{
> + char *shell;
> +
> + set_content (
> + "/bin/foo/bar/ll/pants\n"
> + );
> +
> + shell = getusershell ();
> + if (shell == NULL || strcmp (shell, "/bin/foo/bar/ll/pants") != 0)
> + {
> + dprintf ("shell should be /bin/foo/bar/ll/pants, not '%s'\n", shell);
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +do_test (void)
> +{
> + path_shells_fd = create_temp_file ("getusershell", &path_shells);
> + if (path_shells_fd < 0)
> + return 1;
> +
> + return
> + check_default () +
> + check_empty () +
> + check_comments_blank_lines () +
> + check_valid_shells ();
> +}
>
@@ -22,6 +22,10 @@ Version 2.23
19049, 19050, 19059, 19071, 19074, 19076, 19077, 19078, 19079, 19085,
19086, 19088, 19094, 19095, 19124, 19125, 19129, 19134, 19137.
+* The getusershell function now returns entries from /etc/shells only when
+ the line starts with a '/', and no longer chops lines based on '#' or white
+ space.
+
* There is now a --disable-timezone-tools configure option for disabling the
building and installing of the timezone related utilities (zic, zdump, and
tzselect). This is useful for people who build the timezone data and code
@@ -77,7 +77,8 @@ gpl2lgpl := error.c error.h
tests := tst-dirname tst-tsearch tst-fdset tst-efgcvt tst-mntent tst-hsearch \
tst-error1 tst-pselect tst-insremque tst-mntent2 bug-hsearch1 \
- tst-mntent-blank-corrupt tst-mntent-blank-passno
+ tst-mntent-blank-corrupt tst-mntent-blank-passno tst-getusershell \
+ tst-endusershell
ifeq ($(run-built-tests),yes)
tests-special += $(objpfx)tst-error1-mem.out
endif
@@ -1,143 +1,112 @@
-/*
- * Copyright (c) 1985, 1993
- * The Regents of the University of California. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 4. Neither the name of the University nor the names of its contributors
- * may be used to endorse or promote products derived from this software
- * without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-#if defined(LIBC_SCCS) && !defined(lint)
-static char sccsid[] = "@(#)getusershell.c 8.1 (Berkeley) 6/4/93";
-#endif /* LIBC_SCCS and not lint */
-
-#include <sys/param.h>
-#include <sys/file.h>
-#include <sys/stat.h>
+/* Copyright (C) 2015 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <assert.h>
+#include <paths.h>
#include <stdio.h>
-#include <stdio_ext.h>
-#include <ctype.h>
#include <stdlib.h>
+#include <string.h>
#include <unistd.h>
-#include <paths.h>
-/*
- * Local shells should NOT be added here. They should be added in
- * /etc/shells.
- */
-
-/* NB: we do not initialize okshells here. The initialization needs
- relocations. These interfaces are used so rarely that this is not
- justified. Instead explicitly initialize the array when it is
- used. */
-#if 0
-static const char *const okshells[] = { _PATH_BSHELL, _PATH_CSHELL, NULL };
-#else
-static const char *okshells[3];
+/* Used by tests to point to custom paths. */
+#ifndef _LOCAL_PATH_SHELLS
+# define _LOCAL_PATH_SHELLS _PATH_SHELLS
#endif
-static char **curshell, **shells, *strings;
-static char **initshells (void) __THROW;
-/*
- * Get a list of shells from _PATH_SHELLS, if it exists.
- */
+/* These functions are not thread safe (by design), so globals are OK. */
+static FILE *shellfp;
+static int okshell_idx;
+static char *shellbuf;
+static size_t buf_len;
+
char *
getusershell (void)
{
- char *ret;
-
- if (curshell == NULL)
- curshell = initshells();
- ret = *curshell;
- if (ret != NULL)
- curshell++;
- return (ret);
+ /* Handle the builtin case first.
+
+ NB: we do not use a global array here as the initialization needs
+ relocations. These interfaces are used so rarely that this is not
+ justified. Instead open code it with a switch statement. */
+ switch (okshell_idx)
+ {
+ case 0:
+ if (shellfp == NULL)
+ {
+ setusershell ();
+ return getusershell ();
+ }
+ break;
+ case 1:
+ okshell_idx++;
+ return (char *) _PATH_BSHELL;
+ case 2:
+ okshell_idx++;
+ return (char *) _PATH_CSHELL;
+ case 3:
+ return NULL;
+ }
+
+ /* Walk the /etc/shells file. */
+ while (1)
+ {
+ ssize_t len;
+ char *p;
+
+ len = getline (&shellbuf, &buf_len, shellfp);
+ if (len <= 0)
+ break;
+
+ /* Chop the trailing newline. */
+ p = shellbuf + len - 1;
+ if (*p == '\n')
+ *p = '\0';
+
+ /* Only accept valid shells (which we define as starting with a '/'). */
+ if (shellbuf[0] == '/')
+ return shellbuf;
+ }
+
+ /* No more valid shells. */
+ return NULL;
}
+hidden_proto (endusershell);
void
endusershell (void)
{
-
- free(shells);
- shells = NULL;
- free(strings);
- strings = NULL;
- curshell = NULL;
+ okshell_idx = 0;
+ if (shellfp)
+ {
+ fclose (shellfp);
+ shellfp = NULL;
+ free (shellbuf);
+ shellbuf = NULL;
+ }
}
+hidden_def (endusershell);
void
setusershell (void)
{
+ /* We could rewind shellfp here, but we get smaller code this way.
+ Keep in mind this is not a performance sensitive API. */
+ endusershell ();
- curshell = initshells();
-}
-
-static char **
-initshells (void)
-{
- char **sp, *cp;
- FILE *fp;
- struct stat64 statb;
- size_t flen;
-
- free(shells);
- shells = NULL;
- free(strings);
- strings = NULL;
- if ((fp = fopen(_PATH_SHELLS, "rce")) == NULL)
- goto init_okshells_noclose;
- if (fstat64(fileno(fp), &statb) == -1) {
- init_okshells:
- (void)fclose(fp);
- init_okshells_noclose:
- okshells[0] = _PATH_BSHELL;
- okshells[1] = _PATH_CSHELL;
- return (char **) okshells;
- }
- if (statb.st_size > ~(size_t)0 / sizeof (char *) * 3)
- goto init_okshells;
- flen = statb.st_size + 3;
- if ((strings = malloc(flen)) == NULL)
- goto init_okshells;
- shells = malloc(statb.st_size / 3 * sizeof (char *));
- if (shells == NULL) {
- free(strings);
- strings = NULL;
- goto init_okshells;
- }
- sp = shells;
- cp = strings;
- while (fgets_unlocked(cp, flen - (cp - strings), fp) != NULL) {
- while (*cp != '#' && *cp != '/' && *cp != '\0')
- cp++;
- if (*cp == '#' || *cp == '\0' || cp[1] == '\0')
- continue;
- *sp++ = cp;
- while (!isspace(*cp) && *cp != '#' && *cp != '\0')
- cp++;
- *cp++ = '\0';
- }
- *sp = NULL;
- (void)fclose(fp);
- return (shells);
+ shellfp = fopen (_LOCAL_PATH_SHELLS, "rce");
+ if (shellfp == NULL)
+ okshell_idx = 1;
}
new file mode 100644
@@ -0,0 +1,64 @@
+/* Make sure calling endusershell first doesn't crash.
+
+ Copyright (C) 2015 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+static int do_test (void);
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
+
+static int
+do_test (void)
+{
+ /* This should not crash or corrupt. */
+ endusershell ();
+ endusershell ();
+ endusershell ();
+
+ /* Neither should this. */
+ setusershell ();
+ setusershell ();
+ setusershell ();
+ setusershell ();
+ setusershell ();
+
+ /* Shouldn't be a problem, but we can't really verify the return since
+ it depends on /etc/shells in the installed system. */
+ getusershell ();
+ getusershell ();
+ getusershell ();
+ getusershell ();
+ getusershell ();
+ getusershell ();
+
+ /* Test resetting behavior. */
+ setusershell ();
+ getusershell ();
+ setusershell ();
+ getusershell ();
+
+ /* Test circular behavior. */
+ setusershell ();
+ getusershell ();
+ endusershell ();
+ setusershell ();
+ getusershell ();
+ endusershell ();
+
+ return 0;
+}
new file mode 100644
@@ -0,0 +1,178 @@
+/* Verify behavior of getusershell w/various inputs.
+
+ Copyright (C) 2015 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+/* Set the functions to read from our custom file so we can control the
+ exact content for the tests. */
+static char *path_shells;
+#define _LOCAL_PATH_SHELLS path_shells
+#include "getusershell.c"
+
+#include <assert.h>
+
+static int do_test (void);
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
+
+static int path_shells_fd;
+
+static void
+set_content (const char *content)
+{
+ size_t len = strlen (content);
+
+ assert (lseek (path_shells_fd, 0, SEEK_SET) == 0);
+ assert (ftruncate (path_shells_fd, 0) == 0);
+ assert (write (path_shells_fd, content, len) == len);
+
+ /* Reset the shell reading state since we rewrote the file. */
+ endusershell ();
+}
+
+#define dprintf(fmt, args...) printf ("%s: " fmt, __func__, ## args)
+
+/* Verify behavior of getusershell w/out any /etc/shells file. */
+static int
+check_default (void)
+{
+ char bad_shells[] = "";
+ char *old_shells = path_shells;
+ const char *shell;
+ int ret = 1, i;
+
+ /* This will cause fopen to fail. */
+ path_shells = bad_shells;
+
+ shell = getusershell ();
+ if (shell == NULL || strcmp (shell, "/bin/sh") != 0)
+ {
+ dprintf ("first shell must be /bin/sh, not '%s'\n", shell);
+ goto done;
+ }
+
+ shell = getusershell ();
+ if (shell == NULL || strcmp (shell, "/bin/csh") != 0)
+ {
+ dprintf ("second shell must be /bin/csh, not '%s'\n", shell);
+ goto done;
+ }
+
+ /* Call it a few times to make sure it doesn't wrap or anything. */
+ for (i = 0; i < 10; ++i)
+ {
+ shell = getusershell ();
+ if (shell != NULL)
+ {
+ dprintf ("third shell must be NULL, not '%s'\n", shell);
+ goto done;
+ }
+ }
+
+ ret = 0;
+ done:
+ path_shells = old_shells;
+ return ret;
+}
+
+/* An empty file should return NULL forever. */
+static int
+check_empty (void)
+{
+ int i;
+ char *shell;
+
+ set_content ("");
+
+ for (i = 0; i < 10; ++i)
+ {
+ shell = getusershell ();
+ if (shell != NULL)
+ {
+ dprintf ("empty file should return NULL, not '%s'\n", shell);
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+/* Verify we handle comments and blank lines correctly. */
+static int
+check_comments_blank_lines (void)
+{
+ int i;
+ char *shell;
+
+ set_content (
+ /* A mix of comments and blanks. */
+ "# A comment\n"
+ " # A comment w/leading spaces\n"
+ "\t\t# A comment w/leading tabs\n"
+ /* Now just blank lines. */
+ "\n"
+ " \n"
+ "\t \t\n"
+ );
+
+ for (i = 0; i < 10; ++i)
+ {
+ shell = getusershell ();
+ if (shell != NULL)
+ {
+ dprintf ("comment/blank file should return NULL, not '%s'\n", shell);
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+/* Verify "valid" shells which is defined by starting with a /. */
+static int
+check_valid_shells (void)
+{
+ char *shell;
+
+ set_content (
+ "/bin/foo/bar/ll/pants\n"
+ );
+
+ shell = getusershell ();
+ if (shell == NULL || strcmp (shell, "/bin/foo/bar/ll/pants") != 0)
+ {
+ dprintf ("shell should be /bin/foo/bar/ll/pants, not '%s'\n", shell);
+ return 1;
+ }
+
+ return 0;
+}
+
+static int
+do_test (void)
+{
+ path_shells_fd = create_temp_file ("getusershell", &path_shells);
+ if (path_shells_fd < 0)
+ return 1;
+
+ return
+ check_default () +
+ check_empty () +
+ check_comments_blank_lines () +
+ check_valid_shells ();
+}