Discussion:
pax name_split() error
Peter Bisroev
2016-02-13 04:48:11 UTC
Permalink
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/sp4444/sp5555/this_name_including_path_is_101_bytes_long____________________
pax: File name too long for ustar /pax_test/sp1/sp22/sp333/sp4444/sp5555/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 -0000 1.58
+++ tar.c 13 Feb 2016 04:04:40 -0000
@@ -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);

/*
Otto Moerbeek
2016-02-13 07:45:05 UTC
Permalink
Post by 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/sp4444/sp5555/this_name_including_path_is_101_bytes_long____________________
pax: File name too long for ustar /pax_test/sp1/sp22/sp333/sp4444/sp5555/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=145495481100003&r=1&w=2
Post by Peter Bisroev
Also, there is another tricky issue which still exhibits proper behavior
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
Post by Peter Bisroev
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 -0000 1.58
+++ tar.c 13 Feb 2016 04:04:40 -0000
@@ -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);
/*
Peter Bisroev
2016-02-13 14:01:06 UTC
Permalink
Post by Otto Moerbeek
...
http://marc.info/?t=145495481100003&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?
Post by Otto Moerbeek
Post by Peter Bisroev
Also, there is another tricky issue which still exhibits proper behavior
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

Loading...