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/23 13:17:41 UTC
svn commit: r239390 - in /apr/apr/trunk: CHANGES test/testuser.c
user/unix/groupinfo.c user/unix/userinfo.c
Author: jorton
Date: Tue Aug 23 04:17:36 2005
New Revision: 239390
URL: http://svn.apache.org/viewcvs?rev=239390&view=rev
Log:
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).
Modified:
apr/apr/trunk/CHANGES
apr/apr/trunk/test/testuser.c
apr/apr/trunk/user/unix/groupinfo.c
apr/apr/trunk/user/unix/userinfo.c
Modified: apr/apr/trunk/CHANGES
URL: http://svn.apache.org/viewcvs/apr/apr/trunk/CHANGES?rev=239390&r1=239389&r2=239390&view=diff
==============================================================================
--- apr/apr/trunk/CHANGES (original)
+++ apr/apr/trunk/CHANGES Tue Aug 23 04:17:36 2005
@@ -1,5 +1,8 @@
Changes for APR 1.3.0
+ *) Fix error handling where apr_uid_* and apr_gid_* could return
+ APR_SUCCESS in failure cases. [Joe Orton]
+
*) Fix apr_file_gets() and apr_file_read() to catch write failures
when flushing pending writes for a buffered file. [Joe Orton]
Modified: apr/apr/trunk/test/testuser.c
URL: http://svn.apache.org/viewcvs/apr/apr/trunk/test/testuser.c?rev=239390&r1=239389&r2=239390&view=diff
==============================================================================
--- apr/apr/trunk/test/testuser.c (original)
+++ apr/apr/trunk/test/testuser.c Tue Aug 23 04:17:36 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/trunk/user/unix/groupinfo.c
URL: http://svn.apache.org/viewcvs/apr/apr/trunk/user/unix/groupinfo.c?rev=239390&r1=239389&r2=239390&view=diff
==============================================================================
--- apr/apr/trunk/user/unix/groupinfo.c (original)
+++ apr/apr/trunk/user/unix/groupinfo.c Tue Aug 23 04:17:36 2005
@@ -36,13 +36,21 @@
#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 @@
#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;
}
Modified: apr/apr/trunk/user/unix/userinfo.c
URL: http://svn.apache.org/viewcvs/apr/apr/trunk/user/unix/userinfo.c?rev=239390&r1=239389&r2=239390&view=diff
==============================================================================
--- apr/apr/trunk/user/unix/userinfo.c (original)
+++ apr/apr/trunk/user/unix/userinfo.c Tue Aug 23 04:17:36 2005
@@ -38,13 +38,23 @@
{
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 @@
}
return errno;
}
+#endif
return APR_SUCCESS;
}