bug#13588: Pax hangs in case big UID

[Petr Hracek]

Hello Jack and Stefano,

Bellow is corrected patch for automake.
Jack thank you for corrections. Now the patch looks like better.


From 98a64a309a0f7271d2772dd63e45e43b1163c315 Mon Sep 17 00:00:00 2001
From: Petr Hracek <address@hidden>
Date: Thu, 21 Mar 2013 13:27:39 +0100
Subject: [PATCH] maint: pax hangs in case big UID

See automake bug #13588

* m4/tar.m4: check for ustar V7 archive format. Maximum value for user 
or group ID
is limited to 2097151. Thanks to Jack Kelly for patch corrections
---
 m4/tar.m4 | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/m4/tar.m4 b/m4/tar.m4
index ec8c83e..9b1d206 100644
--- a/m4/tar.m4
+++ b/m4/tar.m4
@@ -81,6 +81,28 @@ do
   AM_RUN_LOG([tardir=conftest.dir && eval $am__tar_ >conftest.tar])
   rm -rf conftest.dir
   if test -s conftest.tar; then
+    m4_if($1, [ustar],
+    [AC_CHECK_PROG([am_prog_have_id], [id], [yes], [no])
+    if test x"$am_prog_have_id" = x"yes"; then
+      AC_MSG_CHECKING([if uid is small enough for ustar])
+      user_id=`id -u`
+      # Maximum allowed UID in case ustar format is 2097151
+      if test $? -eq 0 -a $user_id -gt 2097151; then
+        AC_MSG_RESULT([no])
+        AC_MSG_ERROR([the uid is too large for ustar-format tarfiles. 
Change format in configure.ac],[2])
+      else
+        AC_MSG_RESULT([yes])
+      fi
+      AC_MSG_CHECKING([if gid is small enough for ustar])
+      group_id=`id -g`
+      # Maximum allowed GID in case ustar format is 2097151
+      if test $? -eq 0 -a $group_id -gt 2097151; then
+        AC_MSG_RESULT([no])
+        AC_MSG_ERROR([the gid is too large for ustar-format tarfiles. 
Change format in configure.ac],[2])
+      else
+        AC_MSG_RESULT([yes])
+      fi
+    fi])
     AM_RUN_LOG([$am__untar <conftest.tar])
     grep GrepMe conftest.dir/file >/dev/null 2>&1 && break
   fi
--
1.8.1.4


On 03/20/2013 01:02 PM, Petr Hracek wrote:
> Hello Jack,
>
> that's sound better than my proposed patched.
> It's shorter and  more understandable.
>
> Yes you are right that user should be informed for testing ustar format.
>
> Best regards
> Petr
>
>
> On 03/20/2013 12:55 PM, Jack Kelly wrote:
> > Hi Petr, I have a couple of observations:
> >
> > - AC_MSG_ERROR is going to stop the configure anyway, so you don't need
> >    exit 1.
> >
> > - I'd suggest the following messages for your AC_MSG_ERRORS:
> >
> >    "the uid is too large for ustar-format tarfiles. Change format in
> >    configure.ac"
> >
> >    "the gid is too large for ustar-format tarfiles. Change format in
> >    configure.ac"
> >
> > - Is there a reason you've gone with "test -ge 2097152" instead of "test
> >    -gt 2097151"?
> >
> > - Test for $1 = ustar first, so you only AC_CHECK_PROG for id when
> >    needed.
> >
> > - You could merge some of those nested tests:
> >
> > if test $? -eq 0 -a $user_id -gt 2097151; then
> >    AC_MSG_ERROR([...])
> > fi
> >
> > - That argument can be conditionally expanded at m4 time, so it'll be a
> >    bit faster for end users to use an m4 conditional here.
> >
> > - In fact, you might want to structure it like this:
> >
> > m4_if($1, [ustar],
> > [AC_CHECK_PROG([am_prog_have_id], [id], [yes], [no])
> > if test x"$am_prog_have_id" = x"yes"; then
> >    AC_MSG_CHECKING([if uid is small enough for ustar])
> >    user_id=`id -u`
> >    if test $? -eq 0 -a $user_id -gt 2097151; then
> >      AC_MSG_RESULT([yes])
> >    else
> >      AC_MSG_RESULT([no])
> >      AC_MSG_ERROR([...])
> >    fi
> >    AC_MSG_CHECKING([if gid is small enough for ustar])
> >    group_id=`id -g`
> >    if test $? -eq 0 -a $group_id -gt 2097151; then
> >      AC_MSG_RESULT([yes])
> >    else
> >      AC_MSG_RESULT([no])
> >      AC_MSG_ERROR([...])
> >    fi
> > fi])
> >
> > But that's just my opinion, and Stefano's the one who vets the patches
> > around here.
> >
> > Stefano: is this the sort of thing that should have
> > AC_MSG_CHECKING/AC_MSG_RESULT pairs? Also, have I got the m4 quoting
> > right?
> >
> > -- Jack
> >
> > Petr Hracek <address@hidden> writes:
> > > Hello Stefano,
> > > diff --git a/m4/tar.m4 b/m4/tar.m4
> > > index ec8c83e..87477f1 100644
> > > --- a/m4/tar.m4
> > > +++ b/m4/tar.m4
> > > @@ -81,6 +81,27 @@ do
> > >     AM_RUN_LOG([tardir=conftest.dir && eval $am__tar_ >conftest.tar])
> > >     rm -rf conftest.dir
> > >     if test -s conftest.tar; then
> > > +    AC_CHECK_PROG([ID_TEST], id, [yes], [no])
> > > +    if test x"$ID_TEST" = x"yes"; then
> > > +      if test x"$1" = x"ustar" ; then
> > > +         user_id=`id -u`
> > > +         if test $? -eq 0; then
> > > +           # Maximum allowed UID in case ustar format is 2097151.
> > > +           if test $user_id -ge 2097152; then
> > > +             AC_MSG_ERROR([The uid is big and not allowed in case of
> > > ustar format. Change format in configure.ac],[2])
> > > +             exit 1
> > > +           fi
> > > +         fi
> > > +         group_id=`id -g`
> > > +         if test $? -eq 0; then
> > > +           # Maximum allowed GID in case ustar format is 2097151
> > > +           if test $group_id -ge 2097152; then
> > > +             AC_MSG_ERROR([The gid is big and not allowed in case of
> > > ustar format. Change format in configure.ac],[2])
> > > +             exit 1
> > > +           fi
> > > +         fi
> > > +      fi
> > > +    fi
> > >       AM_RUN_LOG([$am__untar <conftest.tar])
> > >       grep GrepMe conftest.dir/file >/dev/null 2>&1 && break
> > >     fi
> > >
> > > I would like to help you so that all issues like style, commit
> > > message,will be properly set.
> > > If you agree I will commit that patch in accordance GNU coding style
> > > and HACKING file.
> > >
> > > Best regards
> > > Petr
> > > On 02/05/2013 02:51 PM, Stefano Lattarini wrote:
> > > > Hi Peter and Petr (no typo :-)
> > > >
> > > > FYI, this patch is on my radar, and I intend to consider it (and
> > > > likely integrate and adjusted version of it) in the upcoming minor
> > > > version of Automake (1.13.2).
> > > >
> > > > If you want to speed things up a bit and make the integration
> > > > work easier to us, you can present the patch as the output of
> > > > "git format-patch" rather than of "git diff", and add a clear
> > > > git commit message that explains the rationale and motivation
> > > > for the change.  That would be appreciated.
> > > >
> > > > On 02/05/2013 01:42 PM, Petr Hracek wrote:
> > > > > Hello Peter,
> > > > >
> > > > > no problem, I will wait.
> > > > > AC_SUBST is used for one place instance of the variable
> > > > > so that we will modify (in future) only one row instead of several.
> > > > I don't understand this rationale; and I agree with Peter that the
> > > > AC_SUBST call on AM_BIG_ID is unneeded; this should be just a shell
> > > > variable (and since it is used only internally by the automake
> > > > generated code, it should likely be renamed to something like
> > > > 'am_big_id', or even '_am_big_id'.
> > > >
> > > > There are other issues too, but I'll get to them when I'll do a proper
> > > > review.
> > > >
> > > > > I hope that this is not too much general.
> > > > >
> > > > > BR
> > > > > Petr
> > > > Thanks,
> > > >     Stefano

--
S pozdravem / Best regards

Petr Hracek

Red Hat Czech s.r.o.
BaseOS Core Services Brno

Email: address@hidden
Web: www.cz.redhat.com