Package: automake;
Reported by: Petr Hracek <phracek <at> redhat.com>
Date: Wed, 30 Jan 2013 13:33:02 UTC
Severity: normal
Tags: patch
Merged with 8343
Done: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Petr Hracek <phracek <at> redhat.com> To: Jack Kelly <jack <at> jackkelly.name> Cc: 13588 <at> debbugs.gnu.org, Stefano Lattarini <stefano.lattarini <at> gmail.com> Subject: bug#13588: Pax hangs in case big UID Date: Thu, 21 Mar 2013 13:35:18 +0100
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 <phracek <at> redhat.com>
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 <phracek <at> redhat.com> 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: phracek <at> redhat.com
Web: www.cz.redhat.com
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.