[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH 2/2] userspec: warn about '.' separator
|
From: |
Paul Eggert |
|
Subject: |
[PATCH 2/2] userspec: warn about '.' separator |
|
Date: |
Thu, 24 Feb 2022 17:11:59 -0800 |
Problem reported by Dan Jacobson (Bug#44770).
* lib/userspec.c: Don’t include stdbool.h since it’s now in our API.
(parse_user_spec_warn): New function, broken out of parse_user_spec
and with a new PWARN arg.
(parse_user_spec): Use it.
* lib/userspec.h: Include stdbool.h and declare new function.
* tests/test-userspec.c (struct test.in): Now a char array
so that it can be modified.
(T): Make the placeholder a valid test, as that simplifies
the code. Omit NULL placeholder at the end, likewise.
(main): Set up T in the new way, and test that the "." separator
acts like the ":" separator except with a warning if it works.
---
ChangeLog | 14 ++++++++
lib/userspec.c | 26 ++++++++++++---
lib/userspec.h | 8 +++--
tests/test-userspec.c | 77 ++++++++++++++++++++++++++++---------------
4 files changed, 91 insertions(+), 34 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index dda27c7eb9..3499d066e4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,19 @@
2022-02-24 Paul Eggert <eggert@cs.ucla.edu>
+ userspec: warn about '.' separator
+ Problem reported by Dan Jacobson (Bug#44770).
+ * lib/userspec.c: Don’t include stdbool.h since it’s now in our API.
+ (parse_user_spec_warn): New function, broken out of parse_user_spec
+ and with a new PWARN arg.
+ (parse_user_spec): Use it.
+ * lib/userspec.h: Include stdbool.h and declare new function.
+ * tests/test-userspec.c (struct test.in): Now a char array
+ so that it can be modified.
+ (T): Make the placeholder a valid test, as that simplifies
+ the code. Omit NULL placeholder at the end, likewise.
+ (main): Set up T in the new way, and test that the "." separator
+ acts like the ":" separator except with a warning if it works.
+
userspec: no need for static vars
* lib/userspec.c (parse_with_separator): Simplify.
diff --git a/lib/userspec.c b/lib/userspec.c
index e75feb255d..0635bf8426 100644
--- a/lib/userspec.c
+++ b/lib/userspec.c
@@ -22,7 +22,6 @@
/* Specification. */
#include "userspec.h"
-#include <stdbool.h>
#include <stdio.h>
#include <sys/types.h>
#include <pwd.h>
@@ -251,15 +250,18 @@ parse_with_separator (char const *spec, char const
*separator,
Either one might be NULL instead, indicating that it was not
given and the corresponding numeric ID was left unchanged.
- Return NULL if successful, a static error message string if not. */
+ Return NULL if successful, a static error message string if not.
+ If PWARN is null, return NULL instead of a warning;
+ otherwise, set *PWARN to true depending on whether returning a warning. */
char const *
-parse_user_spec (char const *spec, uid_t *uid, gid_t *gid,
- char **username, char **groupname)
+parse_user_spec_warn (char const *spec, uid_t *uid, gid_t *gid,
+ char **username, char **groupname, bool *pwarn)
{
char const *colon = gid ? strchr (spec, ':') : NULL;
char const *error_msg =
parse_with_separator (spec, colon, uid, gid, username, groupname);
+ bool warn = false;
if (gid && !colon && error_msg)
{
@@ -272,12 +274,26 @@ parse_user_spec (char const *spec, uid_t *uid, gid_t *gid,
char const *dot = strchr (spec, '.');
if (dot
&& ! parse_with_separator (spec, dot, uid, gid, username, groupname))
- error_msg = NULL;
+ {
+ warn = true;
+ error_msg = pwarn ? N_("warning: '.' should be ':'") : NULL;
+ }
}
+ if (pwarn)
+ *pwarn = warn;
return error_msg;
}
+/* Like parse_user_spec_warn, but generate only errors; no warnings. */
+
+char const *
+parse_user_spec (char const *spec, uid_t *uid, gid_t *gid,
+ char **username, char **groupname)
+{
+ return parse_user_spec_warn (spec, uid, gid, username, groupname, NULL);
+}
+
#ifdef TEST
# define NULL_CHECK(s) ((s) == NULL ? "(null)" : (s))
diff --git a/lib/userspec.h b/lib/userspec.h
index fcfa4b9b74..7d5d063e7e 100644
--- a/lib/userspec.h
+++ b/lib/userspec.h
@@ -19,10 +19,14 @@
#ifndef USERSPEC_H
# define USERSPEC_H 1
+# include <stdbool.h>
# include <sys/types.h>
-const char *
-parse_user_spec (const char *spec_arg, uid_t *uid, gid_t *gid,
+char const *
+parse_user_spec (char const *spec_arg, uid_t *uid, gid_t *gid,
char **username_arg, char **groupname_arg);
+char const *
+parse_user_spec_warn (char const *spec_arg, uid_t *uid, gid_t *gid,
+ char **username_arg, char **groupname_arg, bool *pwarn);
#endif
diff --git a/tests/test-userspec.c b/tests/test-userspec.c
index c2e36ffb50..65287a592d 100644
--- a/tests/test-userspec.c
+++ b/tests/test-userspec.c
@@ -35,7 +35,7 @@
struct test
{
- const char *in;
+ char in[100];
uid_t uid;
gid_t gid;
const char *user_name;
@@ -48,6 +48,7 @@ static struct test T[] =
{ "", -1, -1, "", "", NULL},
{ ":", -1, -1, "", "", NULL},
{ "+0:+0", 0, 0, "", "", NULL},
+ { "+0:+0", 0, 0, "", "", NULL},
{ ":+1", -1, 1, "", "", NULL},
{ "+1", 1, -1, "", "", NULL},
{ ":+0", -1, 0, "", "", NULL},
@@ -73,9 +74,7 @@ static struct test T[] =
/* "username:" must expand to UID:GID where GID is username's login group
*/
/* Add an entry like the following to the table, if possible.
{ "U_NAME:", UID,GID, U_NAME, G_NAME, NULL}, */
- { NULL, 0, 0, NULL, NULL, ""}, /* place-holder */
-
- { NULL, 0, 0, NULL, NULL, ""}
+ { "" /* placeholder */, -1, -1, "", "", NULL},
};
#define STREQ(a, b) (strcmp (a, b) == 0)
@@ -114,19 +113,14 @@ main (void)
size_t len;
if (!pw || !pw->pw_name || !(gr = getgrgid (pw->pw_gid)) ||
!gr->gr_name)
continue;
- j = ARRAY_CARDINALITY (T) - 2;
- assert (T[j].in == NULL);
- assert (T[j+1].in == NULL);
+ j = ARRAY_CARDINALITY (T) - 1;
len = strlen (pw->pw_name);
+ if (sizeof T[j].in - 2 < len)
+ continue;
/* Store "username:" in T[j].in. */
- {
- char *t = xmalloc (len + 1 + 1);
- memcpy (t, pw->pw_name, len);
- t[len] = ':';
- t[len+1] = '\0';
- T[j].in = t;
- }
+ memcpy(T[j].in, pw->pw_name, len);
+ strcpy(T[j].in + len, ":");
T[j].uid = uid;
T[j].gid = gr->gr_gid;
@@ -137,16 +131,17 @@ main (void)
}
}
- for (i = 0; T[i].in; i++)
+ char *user_name = NULL;
+ char *group_name = NULL;
+
+ for (i = 0; i < ARRAY_CARDINALITY (T); i++)
{
uid_t uid = (uid_t) -1;
gid_t gid = (gid_t) -1;
- char *user_name;
- char *group_name;
- char const *diag = parse_user_spec (T[i].in, &uid, &gid,
- &user_name, &group_name);
free (user_name);
free (group_name);
+ char const *diag = parse_user_spec (T[i].in, &uid, &gid,
+ &user_name, &group_name);
if (!same_diag (diag, T[i].result))
{
printf ("%s return value mismatch: got %s, expected %s\n",
@@ -170,14 +165,42 @@ main (void)
fail = 1;
}
- if (!T[i].result)
- continue;
-
- /* Expected a non-NULL result diagnostic, yet got NULL. */
- diag = "NULL";
- printf ("%s diagnostic mismatch (-: expected diagnostic; +:actual)\n"
- "-%s\n+%s\n", T[i].in, T[i].result, diag);
- fail = 1;
+ if (T[i].result)
+ {
+ /* Expected a non-NULL result diagnostic, yet got NULL. */
+ diag = "NULL";
+ printf ("%s diagnostic mismatch (-: expected diagnostic; +:actual)\n"
+ "-%s\n+%s\n", T[i].in, T[i].result, diag);
+ fail = 1;
+ }
+ else
+ {
+ /* Should get the same result, but with a warning, if replacing
+ ':' with '.'. */
+ char *colon = strchr (T[i].in, ':');
+ if (colon)
+ {
+ *colon = '.';
+ uid_t uid2 = -1;
+ gid_t gid2 = -1;
+ char *user_name2 = NULL;
+ char *group_name2 = NULL;
+ bool warn;
+ if (! (parse_user_spec_warn (T[i].in, &uid2, &gid2,
+ &user_name2, &group_name2, &warn)
+ && warn))
+ printf ("%s did not warn\n", T[i].in);
+ else if (! (uid == uid2 && gid == gid2
+ && !!user_name == !!user_name2
+ && (!user_name || STREQ (user_name, user_name2))
+ && !!group_name == !!group_name2
+ && (!group_name || STREQ (group_name, group_name2))))
+ printf ("%s treated differently than with colon\n", T[i].in);
+
+ free (user_name2);
+ free (group_name2);
+ }
+ }
}
/* Ensure NULL parameters are ignored. */
--
2.35.1