Bring all get{pw,gr}*_r error handling in line with POSIX:

* 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).


git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@239390 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 35 insertions, 8 deletions

diff --git a/user/unix/groupinfo.c b/user/unix/groupinfo.c
index 819af43ec..9f556b20f 100644
--- a/user/unix/groupinfo.c
+++ b/user/unix/groupinfo.c
@@ -36,13 +36,21 @@ APR_DECLARE(apr_status_t) apr_gid_name_get(char **groupname, apr_gid_t groupid,
 #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
     if ((gr = getgrgid(groupid)) == NULL) {
-#endif
         return errno;
     }
+#endif
     *groupname = apr_pstrdup(p, gr->gr_name);
     return APR_SUCCESS;
 }
@@ -55,13 +63,21 @@ APR_DECLARE(apr_status_t) apr_gid_get(apr_gid_t *groupid,
 #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
     if ((gr = getgrnam(groupname)) == NULL) {
-#endif
         return errno;
     }
+#endif
     *groupid = gr->gr_gid;
     return APR_SUCCESS;
 }
diff --git a/user/unix/userinfo.c b/user/unix/userinfo.c
index e049f5e9b..c2a7a1f95 100644
--- a/user/unix/userinfo.c
+++ b/user/unix/userinfo.c
@@ -38,13 +38,23 @@ static apr_status_t getpwnam_safe(const char *username,
 {
     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
     if ((pwptr = getpwnam(username)) != NULL) {
         memcpy(pw, pwptr, sizeof *pw);
-#endif
     }
     else {
         if (errno == 0) {
@@ -53,6 +63,7 @@ static apr_status_t getpwnam_safe(const char *username,
         }
         return errno;
     }
+#endif
     return APR_SUCCESS;
 }