Re: pax name_split() error

2016-02-13 Thread Peter Bisroev
On Sat, Feb 13, 2016 at 2:45 AM, Otto Moerbeek  wrote:
> On Fri, Feb 12, 2016 at 11:48:11PM -0500, Peter Bisroev wrote:
>
> ...
>
> This problem is already being discussed on bugs@ (with a different diff).
> I suggest to send this diff to bugs@ to keep the converation in one place.
> http://marc.info/?t=14549548113&r=1&w=2

Hi Otto, thank you for pointing this out, looking at the submission dates I
see why I have missed that post. I saw this issue last week but did not
file a bug report at that time. So when I did send it through there was one
already. Will double check next time to avoid this race condition :). Sorry,
my fault.

The diff on that thread appears to be the same as mine here, except for the
comments. Should I still send it through or just leave it here and keep that
thread clean?

>> Also, there is another tricky issue which still exhibits proper behavior
>> according to the spec:
>>   
>> http://pubs.opengroup.org/onlinepubs/9699919799/utilities/pax.html#tag_20_92_13_06
>> yet it is not very consistent.
>>
>> If there is a directory name which is exactly 155 bytes long, and this
>> directory is being archived, pax will complain that that directory name is 
>> too
>> long, yet it will archive the files underneath that directory assuming they
>> fit into the TNMSZ name limit.
>>
>> In circumstances like these, what would be the most appropriate behavior for
>> pax? Because if that directory is empty, it will not be archived, but if 
>> there
>> is even a single file underneath it pax will complain, but still archive this
>> directory without an error when it is archiving the actual file.
>>
>> Please let me know if I can help further.
>
> This problem is related but different, I'll Cc: guenther to bring this to
> his attention.
>
> -Otto

Thank you Otto!
--peter



Re: pax name_split() error

2016-02-12 Thread Otto Moerbeek
On Fri, Feb 12, 2016 at 11:48:11PM -0500, Peter Bisroev wrote:

> Dear Developers,
> 
> I believe there is an issue with pax when it tries to archive path names that
> are exactly 101 bytes long and start with a slash. For example (sorry for
> long lines)
> 
> $ pax -x ustar -w -f ./test.tar 
> /pax_test/sp1/sp22/sp333/sp/sp/this_name_including_path_is_101_bytes_long
> pax: File name too long for ustar 
> /pax_test/sp1/sp22/sp333/sp/sp/this_name_including_path_is_101_bytes_long
> 
> I believe the patch below addresses this issue.

This problem is already being discussed on bugs@ (with a different diff).
I suggest to send this diff to bugs@ to keep the converation in one place.
http://marc.info/?t=14549548113&r=1&w=2

> 
> Also, there is another tricky issue which still exhibits proper behavior
> according to the spec:
>   
> http://pubs.opengroup.org/onlinepubs/9699919799/utilities/pax.html#tag_20_92_13_06
> yet it is not very consistent.
> 
> If there is a directory name which is exactly 155 bytes long, and this
> directory is being archived, pax will complain that that directory name is too
> long, yet it will archive the files underneath that directory assuming they
> fit into the TNMSZ name limit.
> 
> In circumstances like these, what would be the most appropriate behavior for
> pax? Because if that directory is empty, it will not be archived, but if there
> is even a single file underneath it pax will complain, but still archive this
> directory without an error when it is archiving the actual file.
> 
> Please let me know if I can help further.

This problem is related but different, I'll Cc: guenther to bring this to
his attention.

-Otto

> 
> Thank you!
> --peter
> 
> 
> 
> Index: tar.c
> ===
> RCS file: /cvs/src/bin/pax/tar.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 tar.c
> --- tar.c 17 Mar 2015 03:23:17 -  1.58
> +++ tar.c 13 Feb 2016 04:04:40 -
> @@ -1159,6 +1159,18 @@ name_split(char *name, int len)
>* include the slash between the two parts that gets thrown away)
>*/
>   start = name + len - TNMSZ - 1;
> + 
> + /*
> +  * If the name is exactly TNMSZ + 1 bytes long and starts with a slash,
> +  * we need to be able to move off the first slash in order to find an
> +  * appropriate splitting point. If the split point is not found, the
> +  * name should not be accepted as per p1003.1-1990 spec for ustar.
> +  * We could force a prefix of / and the file would then expand on
> +  * extract to //name, which is wrong.
> +  */
> + if (start == name && *start == '/')
> + start++;
> +
>   while ((*start != '\0') && (*start != '/'))
>   ++start;
>  
> @@ -1170,13 +1182,7 @@ name_split(char *name, int len)
>   return(NULL);
>   len = start - name;
>  
> - /*
> -  * NOTE: /str where the length of str == TNMSZ can not be stored under
> -  * the p1003.1-1990 spec for ustar. We could force a prefix of / and
> -  * the file would then expand on extract to //str. The len == 0 below
> -  * makes this special case follow the spec to the letter.
> -  */
> - if ((len > TPFSZ) || (len == 0))
> + if (len > TPFSZ)
>   return(NULL);
>  
>   /*



pax name_split() error

2016-02-12 Thread Peter Bisroev
Dear Developers,

I believe there is an issue with pax when it tries to archive path names that
are exactly 101 bytes long and start with a slash. For example (sorry for
long lines)

$ pax -x ustar -w -f ./test.tar 
/pax_test/sp1/sp22/sp333/sp/sp/this_name_including_path_is_101_bytes_long
pax: File name too long for ustar 
/pax_test/sp1/sp22/sp333/sp/sp/this_name_including_path_is_101_bytes_long

I believe the patch below addresses this issue.

Also, there is another tricky issue which still exhibits proper behavior
according to the spec:
  
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/pax.html#tag_20_92_13_06
yet it is not very consistent.

If there is a directory name which is exactly 155 bytes long, and this
directory is being archived, pax will complain that that directory name is too
long, yet it will archive the files underneath that directory assuming they
fit into the TNMSZ name limit.

In circumstances like these, what would be the most appropriate behavior for
pax? Because if that directory is empty, it will not be archived, but if there
is even a single file underneath it pax will complain, but still archive this
directory without an error when it is archiving the actual file.

Please let me know if I can help further.

Thank you!
--peter



Index: tar.c
===
RCS file: /cvs/src/bin/pax/tar.c,v
retrieving revision 1.58
diff -u -p -r1.58 tar.c
--- tar.c   17 Mar 2015 03:23:17 -  1.58
+++ tar.c   13 Feb 2016 04:04:40 -
@@ -1159,6 +1159,18 @@ name_split(char *name, int len)
 * include the slash between the two parts that gets thrown away)
 */
start = name + len - TNMSZ - 1;
+   
+   /*
+* If the name is exactly TNMSZ + 1 bytes long and starts with a slash,
+* we need to be able to move off the first slash in order to find an
+* appropriate splitting point. If the split point is not found, the
+* name should not be accepted as per p1003.1-1990 spec for ustar.
+* We could force a prefix of / and the file would then expand on
+* extract to //name, which is wrong.
+*/
+   if (start == name && *start == '/')
+   start++;
+
while ((*start != '\0') && (*start != '/'))
++start;
 
@@ -1170,13 +1182,7 @@ name_split(char *name, int len)
return(NULL);
len = start - name;
 
-   /*
-* NOTE: /str where the length of str == TNMSZ can not be stored under
-* the p1003.1-1990 spec for ustar. We could force a prefix of / and
-* the file would then expand on extract to //str. The len == 0 below
-* makes this special case follow the spec to the letter.
-*/
-   if ((len > TPFSZ) || (len == 0))
+   if (len > TPFSZ)
return(NULL);
 
/*