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:42:47 UTC
svn commit: r239589 - in /apr/apr/branches/1.2.x: CHANGES test/testuser.c
user/unix/groupinfo.c user/unix/userinfo.c
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);