You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by jo...@apache.org on 2005/08/24 11:56:08 UTC
svn commit: r239592 - in /apr/apr/branches/0.9.x: CHANGES test/testuser.c
user/unix/groupinfo.c user/unix/userinfo.c
Author: jorton
Date: Wed Aug 24 02:56:04 2005
New Revision: 239592
URL: http://svn.apache.org/viewcvs?rev=239592&view=rev
Log:
Merge r219635, r219667, r239390, r239574 from trunk:
* user/unix/userinfo.c (getpwnam_safe, apr_uid_name_get): Fix error
handling; always use the getpwnam_r return value as the error code,
and ignore errno, since POSIX does not require that getpwnam_r sets
errno.
* user/unix/groupinfo.c (apr_gid_name_get, apr_gid_get): Fix error
handling as above; and check for the NULL -> "no entry" cases here
too.
* test/testuser.c (fail_userinfo): Add test cases for error handling
(only one of them actually trips on the bugs in the old code with
glibc).
* user/unix/userinfo.c (getpwnam_safe, apr_uid_name_get): Fix error
handling for platforms which do not set errno on non-threadsafe
get{pw,gr}* failures; always return APR_ENOENT for that case.
* user/unix/groupinfo.c (apr_gid_name_get, apr_gid_get): Likewise.
PR: 34053
Submitted by: pquerna, jorton
Modified:
apr/apr/branches/0.9.x/CHANGES
apr/apr/branches/0.9.x/test/testuser.c
apr/apr/branches/0.9.x/user/unix/groupinfo.c
apr/apr/branches/0.9.x/user/unix/userinfo.c
Modified: apr/apr/branches/0.9.x/CHANGES
URL: http://svn.apache.org/viewcvs/apr/apr/branches/0.9.x/CHANGES?rev=239592&r1=239591&r2=239592&view=diff
==============================================================================
--- apr/apr/branches/0.9.x/CHANGES (original)
+++ apr/apr/branches/0.9.x/CHANGES Wed Aug 24 02:56:04 2005
@@ -1,5 +1,9 @@
Changes with APR 0.9.7
+ *) Fix error handling where apr_uid_* and apr_gid_* could segfault
+ or return APR_SUCCESS in failure cases. PR 34053. [Joe Orton,
+ Paul Querna]
+
*) Refactor Win32 condition variables code to address bugs 27654, 34336.
[Henry Jen <henryjen ztune.net>, E Holyat <eholyat yahoo.com>]
Modified: apr/apr/branches/0.9.x/test/testuser.c
URL: http://svn.apache.org/viewcvs/apr/apr/branches/0.9.x/test/testuser.c?rev=239592&r1=239591&r2=239592&view=diff
==============================================================================
--- apr/apr/branches/0.9.x/test/testuser.c (original)
+++ apr/apr/branches/0.9.x/test/testuser.c Wed Aug 24 02:56:04 2005
@@ -93,6 +93,51 @@
CuAssertIntEquals(tc, APR_SUCCESS, apr_gid_compare(gid, retreived_gid));
}
+
+static void fail_userinfo(CuTest *tc)
+{
+ apr_uid_t uid;
+ apr_gid_t gid;
+ apr_status_t rv;
+ char *tmp;
+
+ errno = 0;
+ gid = uid = 9999999;
+ tmp = NULL;
+ rv = apr_uid_name_get(&tmp, uid, p);
+ CuAssert(tc, "apr_uid_name_get should fail or "
+ "return a user name",
+ rv != APR_SUCCESS || tmp != NULL);
+
+ errno = 0;
+ tmp = NULL;
+ rv = apr_gid_name_get(&tmp, gid, p);
+ CuAssert(tc, "apr_gid_name_get should fail or "
+ "return a group name",
+ rv != APR_SUCCESS || tmp != NULL);
+
+ gid = 424242;
+ errno = 0;
+ rv = apr_gid_get(&gid, "I_AM_NOT_A_GROUP", p);
+ CuAssert(tc, "apr_gid_get should fail or "
+ "set a group number",
+ rv != APR_SUCCESS || gid == 424242);
+
+ gid = uid = 424242;
+ errno = 0;
+ rv = apr_uid_get(&uid, &gid, "I_AM_NOT_A_USER", p);
+ CuAssert(tc, "apr_gid_get should fail or "
+ "set a user and group number",
+ rv != APR_SUCCESS || uid == 424242 || gid == 4242442);
+
+ errno = 0;
+ tmp = NULL;
+ rv = apr_uid_homepath_get(&tmp, "I_AM_NOT_A_USER", p);
+ CuAssert(tc, "apr_uid_homepath_get should fail or "
+ "set a path name",
+ rv != APR_SUCCESS || tmp != NULL);
+}
+
#else
static void users_not_impl(CuTest *tc)
{
@@ -110,6 +155,7 @@
SUITE_ADD_TEST(suite, uid_current);
SUITE_ADD_TEST(suite, username);
SUITE_ADD_TEST(suite, groupname);
+ SUITE_ADD_TEST(suite, fail_userinfo);
#endif
return suite;
Modified: apr/apr/branches/0.9.x/user/unix/groupinfo.c
URL: http://svn.apache.org/viewcvs/apr/apr/branches/0.9.x/user/unix/groupinfo.c?rev=239592&r1=239591&r2=239592&view=diff
==============================================================================
--- apr/apr/branches/0.9.x/user/unix/groupinfo.c (original)
+++ apr/apr/branches/0.9.x/user/unix/groupinfo.c Wed Aug 24 02:56:04 2005
@@ -37,13 +37,22 @@
#if APR_HAS_THREADS && defined(_POSIX_THREAD_SAFE_FUNCTIONS) && defined(HAVE_GETGRGID_R)
struct group grp;
char grbuf[512];
+ apr_status_t rv;
- if (getgrgid_r(groupid, &grp, grbuf, sizeof(grbuf), &gr)) {
+ /* See comment in getpwnam_safe on error handling. */
+ rv = getgrgid_r(groupid, &grp, grbuf, sizeof(grbuf), &gr);
+ if (rv) {
+ return rv;
+ }
+ if (gr == NULL) {
+ return APR_ENOENT;
+ }
#else
+ errno = 0;
if ((gr = getgrgid(groupid)) == NULL) {
-#endif
- return errno;
+ return errno ? errno : APR_ENOENT;
}
+#endif
*groupname = apr_pstrdup(p, gr->gr_name);
#endif
return APR_SUCCESS;
@@ -58,13 +67,22 @@
#if APR_HAS_THREADS && defined(_POSIX_THREAD_SAFE_FUNCTIONS) && defined(HAVE_GETGRNAM_R)
struct group grp;
char grbuf[512];
+ apr_status_t rv;
- if (getgrnam_r(groupname, &grp, grbuf, sizeof(grbuf), &gr)) {
+ /* See comment in getpwnam_safe on error handling. */
+ rv = getgrnam_r(groupname, &grp, grbuf, sizeof(grbuf), &gr);
+ if (rv) {
+ return rv;
+ }
+ if (gr == NULL) {
+ return APR_ENOENT;
+ }
#else
+ errno = 0;
if ((gr = getgrnam(groupname)) == NULL) {
-#endif
- return errno;
+ return errno ? errno : APR_ENOENT;
}
+#endif
*groupid = gr->gr_gid;
#endif
return APR_SUCCESS;
Modified: apr/apr/branches/0.9.x/user/unix/userinfo.c
URL: http://svn.apache.org/viewcvs/apr/apr/branches/0.9.x/user/unix/userinfo.c?rev=239592&r1=239591&r2=239592&view=diff
==============================================================================
--- apr/apr/branches/0.9.x/user/unix/userinfo.c (original)
+++ apr/apr/branches/0.9.x/user/unix/userinfo.c Wed Aug 24 02:56:04 2005
@@ -38,21 +38,31 @@
{
struct passwd *pwptr;
#if APR_HAS_THREADS && defined(_POSIX_THREAD_SAFE_FUNCTIONS) && defined(HAVE_GETPWNAM_R)
- /* IRIX getpwnam_r() returns 0 and sets pwptr to NULL on failure */
- if (!getpwnam_r(username, pw, pwbuf, PWBUF_SIZE, &pwptr) && pwptr) {
- /* nothing extra to do on success */
+ apr_status_t rv;
+
+ /* POSIX defines getpwnam_r() et al to return the error number
+ * rather than set errno, and requires pwptr to be set to NULL if
+ * the entry is not found, imply that "not found" is not an error
+ * condition; some implementations do return 0 with pwptr set to
+ * NULL. */
+ rv = getpwnam_r(username, pw, pwbuf, PWBUF_SIZE, &pwptr);
+ if (rv) {
+ return rv;
+ }
+ if (pwptr == NULL) {
+ return APR_ENOENT;
+ }
#else
+ /* Some platforms (e.g. FreeBSD 4.x) do not set errno on NULL "not
+ * found" return values for the non-threadsafe function either. */
+ errno = 0;
if ((pwptr = getpwnam(username)) != NULL) {
memcpy(pw, pwptr, sizeof *pw);
-#endif
}
else {
- if (errno == 0) {
- /* this can happen with getpwnam() on FreeBSD 4.3 */
- return APR_EGENERAL;
- }
- return errno;
+ return errno ? errno : APR_ENOENT;
}
+#endif
return APR_SUCCESS;
}
@@ -114,13 +124,23 @@
#if APR_HAS_THREADS && defined(_POSIX_THREAD_SAFE_FUNCTIONS) && defined(HAVE_GETPWUID_R)
struct passwd pwd;
char pwbuf[PWBUF_SIZE];
+ apr_status_t rv;
+
+ rv = getpwuid_r(userid, &pwd, pwbuf, sizeof(pwbuf), &pw);
+ if (rv) {
+ return rv;
+ }
+
+ if (pw == NULL) {
+ return APR_ENOENT;
+ }
- if (getpwuid_r(userid, &pwd, pwbuf, sizeof(pwbuf), &pw)) {
#else
+ errno = 0;
if ((pw = getpwuid(userid)) == NULL) {
-#endif
- return errno;
+ return errno ? errno : APR_ENOENT;
}
+#endif
*username = apr_pstrdup(p, pw->pw_name);
return APR_SUCCESS;
}