Author: jorton
Date: Wed Aug 24 02:42:42 2005
New Revision: 239589

URL: http://svn.apache.org/viewcvs?rev=239589&view=rev
Log:
Merge r239390, r239574 from trunk:

* user/unix/userinfo.c (getpwnam_safe): 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.


Modified:
    apr/apr/branches/1.2.x/CHANGES
    apr/apr/branches/1.2.x/test/testuser.c
    apr/apr/branches/1.2.x/user/unix/groupinfo.c
    apr/apr/branches/1.2.x/user/unix/userinfo.c

Modified: apr/apr/branches/1.2.x/CHANGES
URL: http://svn.apache.org/viewcvs/apr/apr/branches/1.2.x/CHANGES?rev=239589&r1=239588&r2=239589&view=diff
==============================================================================
--- apr/apr/branches/1.2.x/CHANGES (original)
+++ apr/apr/branches/1.2.x/CHANGES Wed Aug 24 02:42:42 2005
@@ -1,3 +1,8 @@
+Changes for APR 1.2.2
+
+  *) Fix error handling where apr_uid_* and apr_gid_* could return
+     APR_SUCCESS in failure cases.  PR 34053 continued.  [Joe Orton]
+
 Changes for APR 1.2.1
 
   *) Refactor Win32 condition variables code to address bugs 27654, 34336.

Modified: apr/apr/branches/1.2.x/test/testuser.c
URL: http://svn.apache.org/viewcvs/apr/apr/branches/1.2.x/test/testuser.c?rev=239589&r1=239588&r2=239589&view=diff
==============================================================================
--- apr/apr/branches/1.2.x/test/testuser.c (original)
+++ apr/apr/branches/1.2.x/test/testuser.c Wed Aug 24 02:42:42 2005
@@ -93,6 +93,51 @@
     APR_ASSERT_SUCCESS(tc, "apr_gid_compare failed",
                        apr_gid_compare(gid, retreived_gid));
 }
+
+static void fail_userinfo(abts_case *tc, void *data)
+{
+    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);
+    ABTS_ASSERT(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);
+    ABTS_ASSERT(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);
+    ABTS_ASSERT(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);
+    ABTS_ASSERT(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);
+    ABTS_ASSERT(tc, "apr_uid_homepath_get should fail or "
+                "set a path name",
+                rv != APR_SUCCESS || tmp != NULL);
+}
+
 #else
 static void users_not_impl(abts_case *tc, void *data)
 {
@@ -110,6 +155,7 @@
     abts_run_test(suite, uid_current, NULL);
     abts_run_test(suite, username, NULL);
     abts_run_test(suite, groupname, NULL);
+    abts_run_test(suite, fail_userinfo, NULL);
 #endif
 
     return suite;

Modified: apr/apr/branches/1.2.x/user/unix/groupinfo.c
URL: http://svn.apache.org/viewcvs/apr/apr/branches/1.2.x/user/unix/groupinfo.c?rev=239589&r1=239588&r2=239589&view=diff
==============================================================================
--- apr/apr/branches/1.2.x/user/unix/groupinfo.c (original)
+++ apr/apr/branches/1.2.x/user/unix/groupinfo.c Wed Aug 24 02:42:42 2005
@@ -36,13 +36,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) || gr == NULL) {
+    /* 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);
     return APR_SUCCESS;
 }
@@ -55,13 +64,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;
     return APR_SUCCESS;
 }

Modified: apr/apr/branches/1.2.x/user/unix/userinfo.c
URL: http://svn.apache.org/viewcvs/apr/apr/branches/1.2.x/user/unix/userinfo.c?rev=239589&r1=239588&r2=239589&view=diff
==============================================================================
--- apr/apr/branches/1.2.x/user/unix/userinfo.c (original)
+++ apr/apr/branches/1.2.x/user/unix/userinfo.c Wed Aug 24 02:42:42 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;
 }
 
@@ -126,8 +136,9 @@
     }
 
 #else
+    errno = 0;
     if ((pw = getpwuid(userid)) == NULL) {
-        return errno;
+        return errno ? errno : APR_ENOENT;
     }
 #endif
     *username = apr_pstrdup(p, pw->pw_name);