[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
{un,}setenv fixes
|
From: |
Eric Blake |
|
Subject: |
{un,}setenv fixes |
|
Date: |
Mon, 16 Nov 2009 18:01:59 +0000 (UTC) |
|
User-agent: |
Loom/3.14 (http://gmane.org/) |
Coreutils got recent reports of a FreeBSD failure on 'env -u a=b', which should
fail because unsetenv should reject invalid environment variable names (kind of
obvious, though, when you realize BSD unsetenv returns void instead of int,
contrary to the POSIX signature). I think this should fix it, although so far
I have only tested it on mingw and Solaris 8 (both functions are missing),
cygwin 1.5 (where setenv was broken, but unsetenv worked) and cygwin 1.7 and
Solaris 10 (where both functions obey POSIX, at least after today's patch to
cygwin). I'll test on a few more platforms, then push.
From: Eric Blake <address@hidden>
Date: Sat, 14 Nov 2009 22:13:10 -0700
Subject: [PATCH] setenv, unsetenv: work around various bugs
POSIX requires setenv(NULL,"",0), setenv("a=b","",0),
unsetenv(NULL), and unsetenv("a=b") to fail with EINVAL, but
many BSD implementations lack validation. The gnulib
replacement for void unsetenv did not do validation, and the
replacement for setenv was out of sync with glibc. Also, some
BSD implementations of setenv("a","==",1) eat the leading '='.
See also some recent Austin Group interpretations on environ:
http://austingroupbugs.net/view.php?id=167
http://austingroupbugs.net/view.php?id=185
* lib/setenv.c (setenv) [!HAVE_SETENV]: Resync from glibc.
(setenv) [HAVE_SETENV]: Work around bugs.
* lib/unsetenv.c (unsetenv) [HAVE_UNSETENV]: Work around bugs.
* m4/setenv.m4 (gl_FUNC_SETENV_SEPARATE, gl_FUNC_UNSETENV): Check
for bugs.
(gl_FUNC_SETENV): Write in terms of gl_FUNC_SETENV_SEPARATE.
* m4/environ.m4 (gl_ENVIRON): Avoid expand-before-require.
* m4/stdlib_h.m4 (gl_STDLIB_H_DEFAULTS): Update defaults.
* modules/stdlib (Makefile.am): Update substitutions.
* lib/stdlib.in.h (setenv, unsetenv): Update prototypes.
* doc/posix-functions/setenv.texi (setenv): Document the bugs.
* doc/posix-functions/unsetenv.texi (unsetenv): Likewise.
* modules/setenv-tests: New test.
* modules/unsetenv-tests: Likewise.
* tests/test-setenv.c: New file.
* tests/test-unsetenv.c: Likewise.
Signed-off-by: Eric Blake <address@hidden>
---
ChangeLog | 18 ++++++++++
doc/posix-functions/setenv.texi | 11 ++++--
doc/posix-functions/unsetenv.texi | 4 ++
lib/setenv.c | 54 +++++++++++++++++++++++++++++--
lib/stdlib.in.h | 30 ++++++++++++----
lib/unsetenv.c | 27 +++++++++++++++-
m4/environ.m4 | 4 +-
m4/setenv.m4 | 32 ++++++++++++++----
m4/stdlib_h.m4 | 3 +-
modules/setenv-tests | 10 ++++++
modules/stdlib | 3 +-
modules/unsetenv-tests | 11 ++++++
tests/test-setenv.c | 60 ++++++++++++++++++++++++++++++++++
tests/test-unsetenv.c | 65 +++++++++++++++++++++++++++++++++++++
14 files changed, 306 insertions(+), 26 deletions(-)
create mode 100644 modules/setenv-tests
create mode 100644 modules/unsetenv-tests
create mode 100644 tests/test-setenv.c
create mode 100644 tests/test-unsetenv.c
diff --git a/ChangeLog b/ChangeLog
index de93143..08296d9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,23 @@
2009-11-16 Eric Blake <address@hidden>
+ setenv, unsetenv: work around various bugs
+ * lib/setenv.c (setenv) [!HAVE_SETENV]: Resync from glibc.
+ (setenv) [HAVE_SETENV]: Work around bugs.
+ * lib/unsetenv.c (unsetenv) [HAVE_UNSETENV]: Work around bugs.
+ * m4/setenv.m4 (gl_FUNC_SETENV_SEPARATE, gl_FUNC_UNSETENV): Check
+ for bugs.
+ (gl_FUNC_SETENV): Write in terms of gl_FUNC_SETENV_SEPARATE.
+ * m4/environ.m4 (gl_ENVIRON): Avoid expand-before-require.
+ * m4/stdlib_h.m4 (gl_STDLIB_H_DEFAULTS): Update defaults.
+ * modules/stdlib (Makefile.am): Update substitutions.
+ * lib/stdlib.in.h (setenv, unsetenv): Update prototypes.
+ * doc/posix-functions/setenv.texi (setenv): Document the bugs.
+ * doc/posix-functions/unsetenv.texi (unsetenv): Likewise.
+ * modules/setenv-tests: New test.
+ * modules/unsetenv-tests: Likewise.
+ * tests/test-setenv.c: New file.
+ * tests/test-unsetenv.c: Likewise.
+
test-freading: clean up temporary file
* tests/test-freading.c (main): Remove file on success, and use
ASSERT more liberally.
diff --git a/doc/posix-functions/setenv.texi b/doc/posix-functions/setenv.texi
index 7a90cdb..87c9e2d 100644
--- a/doc/posix-functions/setenv.texi
+++ b/doc/posix-functions/setenv.texi
@@ -11,11 +11,16 @@ setenv
@item
This function is missing on some platforms:
AIX 4.3.2, HP-UX 11, IRIX 6.5, Solaris 9, mingw, BeOS.
address@hidden
+On some platforms, this function does not fail with @samp{EINVAL} when
+passed a null pointer, an empty string, or a string containing @samp{=}:
+FreeBSD 6.0, NetBSD 1.6, OpenBSD 3.8, Cygwin 1.5.x.
address@hidden
+On some platforms, this function removes a leading @samp{=} from the
+value argument:
+Cygwin 1.5.x.
@end itemize
Portability problems not fixed by Gnulib:
@itemize
address@hidden
-In some versions of glibc (e.g.@: 2.3.3), @code{setenv} doesn't fail if the
-first argument contains a @samp{=} character.
@end itemize
diff --git a/doc/posix-functions/unsetenv.texi b/doc/posix-
functions/unsetenv.texi
index 04c4fe3..f356627 100644
--- a/doc/posix-functions/unsetenv.texi
+++ b/doc/posix-functions/unsetenv.texi
@@ -15,6 +15,10 @@ unsetenv
This function has the return type @samp{void} instead of @samp{int} on some
platforms:
MacOS X 10.3, FreeBSD 6.0, NetBSD 1.6, OpenBSD 3.8, OSF/1 5.1.
address@hidden
+On some platforms, this function does not fail with @samp{EINVAL} when
+passed a null pointer, an empty string, or a string containing @samp{=}:
+FreeBSD 6.0, NetBSD 1.6, OpenBSD 3.8.
@end itemize
Portability problems not fixed by Gnulib:
diff --git a/lib/setenv.c b/lib/setenv.c
index 83b52b8..0df5b21 100644
--- a/lib/setenv.c
+++ b/lib/setenv.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 1992,1995-1999,2000-2003,2005-2008 Free Software Foundation,
Inc.
+/* Copyright (C) 1992,1995-1999,2000-2003,2005-2009 Free Software Foundation,
Inc.
This file is part of the GNU C Library.
This program is free software: you can redistribute it and/or modify
@@ -32,12 +32,12 @@
# include <unistd.h>
#endif
-#if _LIBC || !HAVE_SETENV
-
#if !_LIBC
# include "malloca.h"
#endif
+#if _LIBC || !HAVE_SETENV
+
#if !_LIBC
# define __environ environ
#endif
@@ -281,6 +281,12 @@ __add_to_environ (const char *name, const char *value,
const char *combined,
int
setenv (const char *name, const char *value, int replace)
{
+ if (name == NULL || *name == '\0' || strchr (name, '=') != NULL)
+ {
+ __set_errno (EINVAL);
+ return -1;
+ }
+
return __add_to_environ (name, value, NULL, replace);
}
@@ -328,3 +334,45 @@ weak_alias (__clearenv, clearenv)
#endif
#endif /* _LIBC || !HAVE_SETENV */
+
+/* The rest of this file is called into use when replacing an existing
+ but buggy setenv. Known bugs include failure to diagnose invalid
+ name, and consuming a leading '=' from value. */
+#if HAVE_SETENV
+
+# undef setenv
+# define STREQ(a, b) (strcmp (a, b) == 0)
+
+int
+rpl_setenv (const char *name, const char *value, int replace)
+{
+ int result;
+ if (!name || !*name || strchr (name, '='))
+ {
+ errno = EINVAL;
+ return -1;
+ }
+ /* Call the real setenv even if replace is 0, in case implementation
+ has underlying data to update, such as when environ changes. */
+ result = setenv (name, value, replace);
+ if (result == 0 && replace && *value == '=')
+ {
+ char *tmp = getenv (name);
+ if (!STREQ (tmp, value))
+ {
+ int saved_errno;
+ size_t len = strlen (value);
+ tmp = malloca (len + 2);
+ /* Since leading '=' is eaten, double it up. */
+ *tmp = '=';
+ memcpy (tmp + 1, value, len + 1);
+ result = setenv (name, tmp, replace);
+ saved_errno = errno;
+ freea (tmp);
+ errno = saved_errno;
+ }
+ }
+ return result;
+}
+
+#endif /* HAVE_SETENV */
diff --git a/lib/stdlib.in.h b/lib/stdlib.in.h
index e2c6bbf..dd15ac0 100644
--- a/lib/stdlib.in.h
+++ b/lib/stdlib.in.h
@@ -384,11 +384,21 @@ extern int rpmatch (const char *response);
#endif
#if @GNULIB_SETENV@
-# if address@hidden@
+# if @REPLACE_SETENV@
+# undef setenv
+# define setenv rpl_setenv
+# endif
+# if address@hidden@ || @REPLACE_SETENV@
/* Set NAME to VALUE in the environment.
If REPLACE is nonzero, overwrite an existing value. */
extern int setenv (const char *name, const char *value, int replace);
# endif
+#elif defined GNULIB_POSIXCHECK
+# undef setenv
+# define setenv(n,v,o) \
+ (GL_LINK_WARNING ("setenv is unportable - " \
+ "use gnulib module setenv for portability"), \
+ setenv (n, v, o))
#endif
#if @GNULIB_STRTOD@
@@ -448,16 +458,20 @@ extern unsigned long long strtoull (const char *string,
char **endptr, int base)
#endif
#if @GNULIB_UNSETENV@
-# if @HAVE_UNSETENV@
-# if @VOID_UNSETENV@
-/* On some systems, unsetenv() returns void.
- This is the case for MacOS X 10.3, FreeBSD 4.8, NetBSD 1.6, OpenBSD 3.4. */
-# define unsetenv(name) ((unsetenv)(name), 0)
-# endif
-# else
+# if @REPLACE_UNSETENV@
+# undef unsetenv
+# define unsetenv rpl_unsetenv
+# endif
+# if address@hidden@ || @REPLACE_UNSETENV@
/* Remove the variable NAME from the environment. */
extern int unsetenv (const char *name);
# endif
+#elif defined GNULIB_POSIXCHECK
+# undef unsetenv
+# define unsetenv(n) \
+ (GL_LINK_WARNING ("unsetenv is unportable - " \
+ "use gnulib module unsetenv for portability"), \
+ unsetenv (n))
#endif
#ifdef __cplusplus
diff --git a/lib/unsetenv.c b/lib/unsetenv.c
index 73ea878..7567011 100644
--- a/lib/unsetenv.c
+++ b/lib/unsetenv.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 1992,1995-1999,2000-2002,2005-2008 Free Software Foundation,
Inc.
+/* Copyright (C) 1992,1995-1999,2000-2002,2005-2009 Free Software Foundation,
Inc.
This file is part of the GNU C Library.
This program is free software: you can redistribute it and/or modify
@@ -47,6 +47,7 @@ __libc_lock_define_initialized (static, envlock)
# define unsetenv __unsetenv
#endif
+#if _LIBC || !HAVE_UNSETENV
int
unsetenv (const char *name)
@@ -88,3 +89,27 @@ unsetenv (const char *name)
# undef unsetenv
weak_alias (__unsetenv, unsetenv)
#endif
+
+#else /* HAVE_UNSETENV */
+
+# undef unsetenv
+
+/* Call the underlying unsetenv, in case there is hidden bookkeeping
+ that needs updating beyond just modifying environ. */
+int
+rpl_unsetenv (const char *name)
+{
+ int result = 0;
+ if (!name || !*name || strchr (name, '='))
+ {
+ errno = EINVAL;
+ return -1;
+ }
+# if !VOID_UNSETENV
+ result =
+# endif
+ unsetenv (name);
+ return result;
+}
+
+#endif /* HAVE_UNSETENV */
diff --git a/m4/environ.m4 b/m4/environ.m4
index b17bb60..1803820 100644
--- a/m4/environ.m4
+++ b/m4/environ.m4
@@ -1,10 +1,10 @@
-# environ.m4 serial 2
+# environ.m4 serial 3
dnl Copyright (C) 2001-2004, 2006-2009 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
dnl with or without modifications, as long as this notice is preserved.
-AC_DEFUN([gl_ENVIRON],
+AC_DEFUN_ONCE([gl_ENVIRON],
[
AC_REQUIRE([gl_UNISTD_H_DEFAULTS])
dnl Persuade glibc <unistd.h> to declare environ.
diff --git a/m4/setenv.m4 b/m4/setenv.m4
index e28407e..ca146d2 100644
--- a/m4/setenv.m4
+++ b/m4/setenv.m4
@@ -1,4 +1,4 @@
-# setenv.m4 serial 11
+# setenv.m4 serial 12
dnl Copyright (C) 2001-2004, 2006-2009 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -6,12 +6,9 @@ dnl with or without modifications, as long as this notice is
preserved.
AC_DEFUN([gl_FUNC_SETENV],
[
- AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
- AC_CHECK_FUNCS_ONCE([setenv])
- if test $ac_cv_func_setenv = no; then
- HAVE_SETENV=0
+ AC_REQUIRE([gl_FUNC_SETENV_SEPARATE])
+ if test $HAVE_SETENV$REPLACE_SETENV != 10; then
AC_LIBOBJ([setenv])
- gl_PREREQ_SETENV
fi
])
@@ -22,6 +19,24 @@ AC_DEFUN([gl_FUNC_SETENV_SEPARATE],
AC_CHECK_FUNCS_ONCE([setenv])
if test $ac_cv_func_setenv = no; then
HAVE_SETENV=0
+ else
+ AC_CACHE_CHECK([whether setenv validates arguments],
+ [gl_cv_func_setenv_works],
+ [AC_RUN_IFELSE([AC_LANG_PROGRAM([[
+ #include <stdlib.h>
+ #include <errno.h>
+ ]], [[
+ if (setenv (NULL, "", 0) != -1) return 1;
+ if (errno != EINVAL) return 2;
+ if (setenv ("a", "=", 1) != 0) return 3;
+ if (strcmp (getenv ("a"), "=") != 0) return 4;
+ ]])],
+ [gl_cv_func_setenv_works=yes], [gl_cv_func_setenv_works=no],
+ [gl_cv_func_setenv_works="guessing no"])])
+ if test "$gl_cv_func_setenv_works" != yes; then
+ REPLACE_SETENV=1
+ AC_LIBOBJ([setenv])
+ fi
fi
gl_PREREQ_SETENV
])
@@ -48,7 +63,10 @@ int unsetenv();
#endif
], , gt_cv_func_unsetenv_ret='int', gt_cv_func_unsetenv_ret='void')])
if test $gt_cv_func_unsetenv_ret = 'void'; then
- VOID_UNSETENV=1
+ AC_DEFINE([VOID_UNSETENV], [1], [Define to 1 if unsetenv returns void
+ instead of int.])
+ REPLACE_UNSETENV=1
+ AC_LIBOBJ([unsetenv])
fi
fi
])
diff --git a/m4/stdlib_h.m4 b/m4/stdlib_h.m4
index 4556ac0..10e010e 100644
--- a/m4/stdlib_h.m4
+++ b/m4/stdlib_h.m4
@@ -80,6 +80,7 @@ AC_DEFUN([gl_STDLIB_H_DEFAULTS],
REPLACE_MKSTEMP=0; AC_SUBST([REPLACE_MKSTEMP])
REPLACE_PUTENV=0; AC_SUBST([REPLACE_PUTENV])
REPLACE_REALPATH=0; AC_SUBST([REPLACE_REALPATH])
+ REPLACE_SETENV=0; AC_SUBST([REPLACE_SETENV])
REPLACE_STRTOD=0; AC_SUBST([REPLACE_STRTOD])
- VOID_UNSETENV=0; AC_SUBST([VOID_UNSETENV])
+ REPLACE_UNSETENV=0; AC_SUBST([REPLACE_UNSETENV])
])
diff --git a/modules/setenv-tests b/modules/setenv-tests
new file mode 100644
index 0000000..9c923ac
--- /dev/null
+++ b/modules/setenv-tests
@@ -0,0 +1,10 @@
+Files:
+tests/test-setenv.c
+
+Depends-on:
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-setenv
+check_PROGRAMS += test-setenv
diff --git a/modules/stdlib b/modules/stdlib
index 2e04088..ddf2b7a 100644
--- a/modules/stdlib
+++ b/modules/stdlib
@@ -73,8 +73,9 @@ stdlib.h: stdlib.in.h
-e 's|@''REPLACE_MKSTEMP''@|$(REPLACE_MKSTEMP)|g' \
-e 's|@''REPLACE_PUTENV''@|$(REPLACE_PUTENV)|g' \
-e 's|@''REPLACE_REALPATH''@|$(REPLACE_REALPATH)|g' \
+ -e 's|@''REPLACE_SETENV''@|$(REPLACE_SETENV)|g' \
-e 's|@''REPLACE_STRTOD''@|$(REPLACE_STRTOD)|g' \
- -e 's|@''VOID_UNSETENV''@|$(VOID_UNSETENV)|g' \
+ -e 's|@''REPLACE_UNSETENV''@|$(REPLACE_UNSETENV)|g' \
-e '/definition of GL_LINK_WARNING/r $(LINK_WARNING_H)' \
< $(srcdir)/stdlib.in.h; \
} > address@hidden && \
diff --git a/modules/unsetenv-tests b/modules/unsetenv-tests
new file mode 100644
index 0000000..94fc504
--- /dev/null
+++ b/modules/unsetenv-tests
@@ -0,0 +1,11 @@
+Files:
+tests/test-unsetenv.c
+
+Depends-on:
+putenv
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-unsetenv
+check_PROGRAMS += test-unsetenv
diff --git a/tests/test-setenv.c b/tests/test-setenv.c
new file mode 100644
index 0000000..61be838
--- /dev/null
+++ b/tests/test-setenv.c
@@ -0,0 +1,60 @@
+/* Tests of setenv.
+ Copyright (C) 2009 Free Software Foundation, Inc.
+
+ This program is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program 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 General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* Written by Eric Blake <address@hidden>, 2009. */
+
+#include <config.h>
+
+#include <stdlib.h>
+
+#include <errno.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+#define ASSERT(expr) \
+ do \
+ { \
+ if (!(expr)) \
+ { \
+ fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \
+ fflush (stderr); \
+ abort (); \
+ } \
+ } \
+ while (0)
+
+int
+main (void)
+{
+ /* Test overwriting. */
+ ASSERT (setenv ("a", "==", -1) == 0);
+ ASSERT (setenv ("a", "2", 0) == 0);
+ ASSERT (strcmp (getenv ("a"), "==") == 0);
+
+ /* Required to fail with EINVAL. */
+ errno = 0;
+ ASSERT (setenv ("", "", 1) == -1);
+ ASSERT (errno == EINVAL);
+ errno = 0;
+ ASSERT (setenv ("a=b", "", 0) == -1);
+ ASSERT (errno == EINVAL);
+ errno = 0;
+ ASSERT (setenv (NULL, "", 0) == -1);
+ ASSERT (errno == EINVAL);
+
+ return 0;
+}
diff --git a/tests/test-unsetenv.c b/tests/test-unsetenv.c
new file mode 100644
index 0000000..11af82c
--- /dev/null
+++ b/tests/test-unsetenv.c
@@ -0,0 +1,65 @@
+/* Tests of unsetenv.
+ Copyright (C) 2009 Free Software Foundation, Inc.
+
+ This program is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program 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 General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* Written by Eric Blake <address@hidden>, 2009. */
+
+#include <config.h>
+
+#include <stdlib.h>
+
+#include <errno.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+#define ASSERT(expr) \
+ do \
+ { \
+ if (!(expr)) \
+ { \
+ fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \
+ fflush (stderr); \
+ abort (); \
+ } \
+ } \
+ while (0)
+
+int
+main (void)
+{
+ char entry[] = "b=2";
+
+ /* Test removal when multiple entries present. */
+ ASSERT (putenv ("a=1") == 0);
+ ASSERT (putenv (entry) == 0);
+ entry[0] = 'a'; /* Unspecified what getenv("a") would be at this point. */
+ ASSERT (unsetenv ("a") == 0); /* Both entries will be removed. */
+ ASSERT (getenv ("a") == NULL);
+ ASSERT (unsetenv ("a") == 0);
+
+ /* Required to fail with EINVAL. */
+ errno = 0;
+ ASSERT (unsetenv ("") == -1);
+ ASSERT (errno == EINVAL);
+ errno = 0;
+ ASSERT (unsetenv ("a=b") == -1);
+ ASSERT (errno == EINVAL);
+ errno = 0;
+ ASSERT (unsetenv (NULL) == -1);
+ ASSERT (errno == EINVAL);
+
+ return 0;
+}
--
1.6.4.2
- {un,}setenv fixes,
Eric Blake <=