svn commit: r195318 - head/usr.bin/cpio

[Tim Kientzle]

Author: kientzle
Date: Fri Jul  3 17:54:33 2009
New Revision: 195318
URL: http://svn.freebsd.org/changeset/base/195318

Log:
  This fixes bsdcpio's -R option to accept numeric
  user or group Ids as well as user or group names.
  In particular, this fixes freesbie2, which uses
  -R 0:0 to copy a bunch of files so that the result
  will be owned by root.
  
  Also fixes a related bug that mixed-up the uid
  and gid specified by -R when in passthrough mode.
  
  Thanks to Dominique Goncalves for reporting this
  regression.
  
  Approved by:  re (kib)

Modified:
  head/usr.bin/cpio/cmdline.c
  head/usr.bin/cpio/cpio.c

Modified: head/usr.bin/cpio/cmdline.c
==
--- head/usr.bin/cpio/cmdline.c Fri Jul  3 16:33:42 2009(r195317)
+++ head/usr.bin/cpio/cmdline.c Fri Jul  3 17:54:33 2009(r195318)
@@ -268,16 +268,36 @@ cpio_getopt(struct cpio *cpio)
  * Parse the argument to the -R or --owner flag.
  *
  * The format is one of the following:
- *   user- Override user but not group
- *   user:   - Override both, group is user's default group
- *   user:group - Override both
- *   :group  - Override group but not user
+ *   username|uid- Override user but not group
+ *   username:   - Override both, group is user's default group
+ *   uid:- Override user but not group
+ *   username|uid:groupname|gid - Override both
+ *   :groupname|gid  - Override group but not user
+ *
+ * Where uid/gid are decimal representations and groupname/username
+ * are names to be looked up in system database.  Note that
+ * uid/gid parsing takes priority over username/groupname lookup,
+ * so this won't do a lookup for usernames or group names that
+ * consist entirely of digits.
  *
  * A period can be used instead of the colon.
  *
- * Sets uid/gid as appropriate, -1 indicates uid/gid not specified.
+ * Sets uid/gid return as appropriate, -1 indicates uid/gid not specified.
  *
  */
+static int
+decimal_parse(const char *p)
+{
+   /* TODO: guard against overflow. */
+   int n = 0;
+   for (; *p != '\0'; ++p) {
+   if (*p  '0' || *p  '9')
+   return (-1);
+   n = n * 10 + *p - '0';
+   }
+   return (n);
+}
+
 int
 owner_parse(const char *spec, int *uid, int *gid)
 {
@@ -318,24 +338,34 @@ owner_parse(const char *spec, int *uid, 
}
memcpy(user, u, ue - u);
user[ue - u] = '\0';
-   pwent = getpwnam(user);
-   if (pwent == NULL) {
-   cpio_warnc(errno, Couldn't lookup user ``%s'', user);
-   return (1);
+   *uid = decimal_parse(user);
+   if (*uid  0) {
+   /* Couldn't parse as integer, try username lookup. */
+   pwent = getpwnam(user);
+   if (pwent == NULL) {
+   cpio_warnc(errno,
+   Couldn't lookup user ``%s'', user);
+   return (1);
+   }
+   *uid = pwent-pw_uid;
+   if (*ue != '\0'  *g == '\0')
+   *gid = pwent-pw_gid;
}
free(user);
-   *uid = pwent-pw_uid;
-   if (*ue != '\0'  *g == '\0')
-   *gid = pwent-pw_gid;
}
if (*g != '\0') {
-   struct group *grp;
-   grp = getgrnam(g);
-   if (grp != NULL)
-   *gid = grp-gr_gid;
-   else {
-   cpio_warnc(errno, Couldn't look up group ``%s'', g);
-   return (1);
+   *gid = decimal_parse(g);
+   if (*gid  0) {
+   /* Couldn't parse int, try group name lookup. */
+   struct group *grp;
+   grp = getgrnam(g);
+   if (grp != NULL)
+   *gid = grp-gr_gid;
+   else {
+   cpio_warnc(errno,
+   Couldn't look up group ``%s'', g);
+   return (1);
+   }
}
}
return (0);

Modified: head/usr.bin/cpio/cpio.c
==
--- head/usr.bin/cpio/cpio.cFri Jul  3 16:33:42 2009(r195317)
+++ head/usr.bin/cpio/cpio.cFri Jul  3 17:54:33 2009(r195318)
@@ -575,7 +575,7 @@ file_to_archive(struct cpio *cpio, const
if (cpio-uid_override = 0)
st.st_uid = cpio-uid_override;
if (cpio-gid_override = 0)
-   st.st_gid = cpio-uid_override;
+   st.st_gid = cpio-gid_override;
archive_entry_copy_stat(entry, st);
 
 #if !defined(_WIN32) || defined(__CYGWIN__)

Re: svn commit: r195318 - head/usr.bin/cpio

[Brooks Davis]

On Fri, Jul 03, 2009 at 05:54:33PM +, Tim Kientzle wrote:
 Modified: head/usr.bin/cpio/cmdline.c
 ==
 --- head/usr.bin/cpio/cmdline.c   Fri Jul  3 16:33:42 2009
 (r195317)
 +++ head/usr.bin/cpio/cmdline.c   Fri Jul  3 17:54:33 2009
 (r195318)
 @@ -268,16 +268,36 @@ cpio_getopt(struct cpio *cpio)
   * Parse the argument to the -R or --owner flag.
   *
   * The format is one of the following:
 - *   user- Override user but not group
 - *   user:   - Override both, group is user's default group
 - *   user:group - Override both
 - *   :group  - Override group but not user
 + *   username|uid- Override user but not group
 + *   username:   - Override both, group is user's default group
 + *   uid:- Override user but not group
 + *   username|uid:groupname|gid - Override both
 + *   :groupname|gid  - Override group but not user
 + *
 + * Where uid/gid are decimal representations and groupname/username
 + * are names to be looked up in system database.  Note that
 + * uid/gid parsing takes priority over username/groupname lookup,
 + * so this won't do a lookup for usernames or group names that
 + * consist entirely of digits.

Is this behavior specified somewhere?  It is counter to the usual rule
that the system should attempt to resolve all strings as a name first.
For example, the following is from the chown manpage in POSIX.1-2008

If a numeric owner operand exists in the user database as a user
name, the user ID number associated with that user name shall be
used as the user ID.

Numeric usernames are a disaster, especially if they can't match the uid
for some reason, but following this rule consistently at least makes
things more sane if you're stuck with them.

-- Brooks


pgpPfi3FmCAGG.pgp
Description: PGP signature

Re: svn commit: r195318 - head/usr.bin/cpio

[Alexey Dokuchaev]

Tim Kientzle wrote:
 Author: kientzle
 Date: Fri Jul  3 17:54:33 2009
 New Revision: 195318
 URL: http://svn.freebsd.org/changeset/base/195318
 
 + * Where uid/gid are decimal representations and groupname/username
 + * are names to be looked up in system database.  Note that ^^^

Bad indentation: uid/gid occupies seven characters, so wrap is
premature here (there are exactly seven ^ marks above).

 + * uid/gid parsing takes priority over username/groupname lookup,
 + * so this won't do a lookup for usernames or group names that
 + * consist entirely of digits.
   *
   * A period can be used instead of the colon.
   *
 - * Sets uid/gid as appropriate, -1 indicates uid/gid not specified.
 + * Sets uid/gid return as appropriate, -1 indicates uid/gid not specified.
   *
   */
 +static int
 +decimal_parse(const char *p)
 +{
 + /* TODO: guard against overflow. */
 + int n = 0;
 + for (; *p != '\0'; ++p) {
 + if (*p  '0' || *p  '9')
 + return (-1);
 + n = n * 10 + *p - '0';
 + }
 + return (n);
 +}

Roll own your own strtoul(3)?  What kind of NIH is this?  :-)

 +
  int
  owner_parse(const char *spec, int *uid, int *gid)
  {
 @@ -318,24 +338,34 @@ owner_parse(const char *spec, int *uid, 
   }
   memcpy(user, u, ue - u);
   user[ue - u] = '\0';
 - pwent = getpwnam(user);
 - if (pwent == NULL) {
 - cpio_warnc(errno, Couldn't lookup user ``%s'', user);
 - return (1);
 + *uid = decimal_parse(user);
 + if (*uid  0) {
 + /* Couldn't parse as integer, try username lookup. */
 + pwent = getpwnam(user);
 + if (pwent == NULL) {
 + cpio_warnc(errno,
 + Couldn't lookup user ``%s'', user);
 + return (1);
 + }
 + *uid = pwent-pw_uid;
 + if (*ue != '\0'  *g == '\0')
 + *gid = pwent-pw_gid;

Why not something simple like this:

if ((pwent = getpwnam(user)) != NULL)
uid = pwent-pw_uid;
else {
errno = 0;
uid = strtoul(user, ep, 10);
if (errno || user[0] == '\0' || *ep != '\0')
cpio_warnc(invalid user: %s, user);
}

./danfe
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org