[aur-dev] [PATCH] pkgsubmit: count actual subdirectories
Rather than relying on a regex, detect directories in the uploaded tarball and count the slashes. This avoids problems with bsdtar inserting PaxHeader attributes into the archive which look something like the following to Archive_Tar: PaxHeader/xcursor-protozoa xcursor-protozoa/ xcursor-protozoa/PaxHeader/PKGBUILD xcursor-protozoa/PKGBUILD This only occurs on certain filesystems (e.g. jfs), but the tarball is by no means invalid. When extracted, it will only contain the PKGBUILD within a single subdirectory. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- web/html/pkgsubmit.php | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/web/html/pkgsubmit.php b/web/html/pkgsubmit.php index 75a4b69..5f5ba30 100644 --- a/web/html/pkgsubmit.php +++ b/web/html/pkgsubmit.php @@ -63,16 +63,13 @@ if ($uid): # Extract PKGBUILD into a string $pkgbuild_raw = ''; - $dircount = 0; foreach ($tar->listContent() as $tar_file) { if (preg_match('/^[^\/]+\/PKGBUILD$/', $tar_file['filename'])) { $pkgbuild_raw = $tar->extractInString($tar_file['filename']); } - elseif (preg_match('/^[^\/]+\/$/', $tar_file['filename'])) { - if (++$dircount > 1) { - $error = __("Error - source tarball may not contain more than one directory."); - break; - } + elseif ($tar_file['filetype'] == 5 && count(explode(',', $tar_file['filename'])) > 1) { + $error = __("Error - source tarball may not contain more than one directory."); + break; } elseif (preg_match('/^[^\/]+$/', $tar_file['filename'])) { $error = __("Error - source tarball may not contain files outside a directory."); -- 1.7.9.4
On Sat, Mar 17, 2012 at 10:35:49PM -0400, Dave Reisner wrote:
Rather than relying on a regex, detect directories in the uploaded tarball and count the slashes. This avoids problems with bsdtar inserting PaxHeader attributes into the archive which look something like the following to Archive_Tar:
PaxHeader/xcursor-protozoa xcursor-protozoa/ xcursor-protozoa/PaxHeader/PKGBUILD xcursor-protozoa/PKGBUILD
This only occurs on certain filesystems (e.g. jfs), but the tarball is by no means invalid. When extracted, it will only contain the PKGBUILD within a single subdirectory.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- web/html/pkgsubmit.php | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/web/html/pkgsubmit.php b/web/html/pkgsubmit.php index 75a4b69..5f5ba30 100644 --- a/web/html/pkgsubmit.php +++ b/web/html/pkgsubmit.php @@ -63,16 +63,13 @@ if ($uid):
# Extract PKGBUILD into a string $pkgbuild_raw = ''; - $dircount = 0; foreach ($tar->listContent() as $tar_file) { if (preg_match('/^[^\/]+\/PKGBUILD$/', $tar_file['filename'])) { $pkgbuild_raw = $tar->extractInString($tar_file['filename']); } - elseif (preg_match('/^[^\/]+\/$/', $tar_file['filename'])) { - if (++$dircount > 1) { - $error = __("Error - source tarball may not contain more than one directory."); - break; - } + elseif ($tar_file['filetype'] == 5 && count(explode(',', $tar_file['filename'])) > 1) {
We don't check for nested subdirectories here, that is done further below (check the last elseif-block). I also doubt that this one is the check that fails since we count the entries that have a trailing "/" here and, looking at the listing in your commit message, there don't seem to be more than one of these. You should probably change the logic in the last elseif condition instead of here... Also, the "," delimiter in explode() seems a bit wrong? :)
+ $error = __("Error - source tarball may not contain more than one directory."); + break; } elseif (preg_match('/^[^\/]+$/', $tar_file['filename'])) { $error = __("Error - source tarball may not contain files outside a directory."); -- 1.7.9.4
On Sun, Mar 18, 2012 at 01:01:59PM +0100, Lukas Fleischer wrote:
On Sat, Mar 17, 2012 at 10:35:49PM -0400, Dave Reisner wrote:
Rather than relying on a regex, detect directories in the uploaded tarball and count the slashes. This avoids problems with bsdtar inserting PaxHeader attributes into the archive which look something like the following to Archive_Tar:
PaxHeader/xcursor-protozoa xcursor-protozoa/ xcursor-protozoa/PaxHeader/PKGBUILD xcursor-protozoa/PKGBUILD
This only occurs on certain filesystems (e.g. jfs), but the tarball is by no means invalid. When extracted, it will only contain the PKGBUILD within a single subdirectory.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- web/html/pkgsubmit.php | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/web/html/pkgsubmit.php b/web/html/pkgsubmit.php index 75a4b69..5f5ba30 100644 --- a/web/html/pkgsubmit.php +++ b/web/html/pkgsubmit.php @@ -63,16 +63,13 @@ if ($uid):
# Extract PKGBUILD into a string $pkgbuild_raw = ''; - $dircount = 0; foreach ($tar->listContent() as $tar_file) { if (preg_match('/^[^\/]+\/PKGBUILD$/', $tar_file['filename'])) { $pkgbuild_raw = $tar->extractInString($tar_file['filename']); } - elseif (preg_match('/^[^\/]+\/$/', $tar_file['filename'])) { - if (++$dircount > 1) { - $error = __("Error - source tarball may not contain more than one directory."); - break; - } + elseif ($tar_file['filetype'] == 5 && count(explode(',', $tar_file['filename'])) > 1) {
We don't check for nested subdirectories here, that is done further below (check the last elseif-block). I also doubt that this one is the check that fails since we count the entries that have a trailing "/" here and, looking at the listing in your commit message, there don't seem to be more than one of these. You should probably change the logic in the last elseif condition instead of here...
Also, the "," delimiter in explode() seems a bit wrong? :)
Epic failure. I think I divided by 0 before sending this patch. d
+ $error = __("Error - source tarball may not contain more than one directory."); + break; } elseif (preg_match('/^[^\/]+$/', $tar_file['filename'])) { $error = __("Error - source tarball may not contain files outside a directory."); -- 1.7.9.4
On Sat 17 Mar 2012 22:35 -0400, Dave Reisner wrote:
Rather than relying on a regex, detect directories in the uploaded tarball and count the slashes. This avoids problems with bsdtar inserting PaxHeader attributes into the archive which look something like the following to Archive_Tar:
PaxHeader/xcursor-protozoa xcursor-protozoa/ xcursor-protozoa/PaxHeader/PKGBUILD xcursor-protozoa/PKGBUILD
This only occurs on certain filesystems (e.g. jfs), but the tarball is by no means invalid. When extracted, it will only contain the PKGBUILD within a single subdirectory.
Just wondering, are these PaxHeaders, as well as Schily tags something that you'd really want to even keep in the archive if they're filesystem dependent? I suppose the AUR should be able to deal with these cases, but it would be nice if makepkg would produce filesystem agnostic source package too. Maybe the AUR should reject them just as well, for the sake of a more consistent format. Of interest: https://bugs.archlinux.org/task/28802 https://www.gnu.org/software/tar/manual/html_section/Portability.html
On Sun, Mar 18, 2012 at 11:11:50AM -0400, Loui Chang wrote:
On Sat 17 Mar 2012 22:35 -0400, Dave Reisner wrote:
Rather than relying on a regex, detect directories in the uploaded tarball and count the slashes. This avoids problems with bsdtar inserting PaxHeader attributes into the archive which look something like the following to Archive_Tar:
PaxHeader/xcursor-protozoa xcursor-protozoa/ xcursor-protozoa/PaxHeader/PKGBUILD xcursor-protozoa/PKGBUILD
This only occurs on certain filesystems (e.g. jfs), but the tarball is by no means invalid. When extracted, it will only contain the PKGBUILD within a single subdirectory.
Just wondering, are these PaxHeaders, as well as Schily tags something that you'd really want to even keep in the archive if they're filesystem dependent? I suppose the AUR should be able to deal with these cases, but it would be nice if makepkg would produce filesystem agnostic source package too.
Maybe the AUR should reject them just as well, for the sake of a more consistent format.
Of interest: https://bugs.archlinux.org/task/28802 https://www.gnu.org/software/tar/manual/html_section/Portability.html
Thanks for the flyspray link. I see no reason for the AUR to reject the tarball -- it's not invalid, and especially not invalid as per the reason given by the error. IMO, we should either fix the error (silly) or simply ignore the headers. d
* Reorder checks. * Use simple string functions instead of regular expressions. * Check for type flags before validating paths. The latter ensures we don't treat tarball keywords/flags as directories. This avoids problems with bsdtar inserting PaxHeader attributes into the archive which look something like the following to Archive_Tar: PaxHeader/xcursor-protozoa xcursor-protozoa/ xcursor-protozoa/PaxHeader/PKGBUILD xcursor-protozoa/PKGBUILD This only occurs on certain filesystems (e.g. jfs), but the tarball is by no means invalid. When extracted, it will only contain the PKGBUILD within a single subdirectory. Addresses FS#28802. Thanks-to: Dave Reisner <dreisner@archlinux.org> Signed-off-by: Lukas Fleischer <archlinux@cryptocrack.de> --- Dave told me to go ahead and fix this. Here we go! web/html/pkgsubmit.php | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/web/html/pkgsubmit.php b/web/html/pkgsubmit.php index 75a4b69..566890b 100644 --- a/web/html/pkgsubmit.php +++ b/web/html/pkgsubmit.php @@ -65,23 +65,25 @@ if ($uid): $pkgbuild_raw = ''; $dircount = 0; foreach ($tar->listContent() as $tar_file) { - if (preg_match('/^[^\/]+\/PKGBUILD$/', $tar_file['filename'])) { - $pkgbuild_raw = $tar->extractInString($tar_file['filename']); + if ($tar_file['typeflag'] == 0) { + if (strchr($tar_file['filename'], '/') === false) { + $error = __("Error - source tarball may not contain files outside a directory."); + break; + } + elseif (substr($tar_file['filename'], -9) == '/PKGBUILD') { + $pkgbuild_raw = $tar->extractInString($tar_file['filename']); + } } - elseif (preg_match('/^[^\/]+\/$/', $tar_file['filename'])) { - if (++$dircount > 1) { + elseif ($tar_file['typeflag'] == 5) { + if (substr_count($tar_file['filename'], "/") > 1) { + $error = __("Error - source tarball may not contain nested subdirectories."); + break; + } + elseif (++$dircount > 1) { $error = __("Error - source tarball may not contain more than one directory."); break; } } - elseif (preg_match('/^[^\/]+$/', $tar_file['filename'])) { - $error = __("Error - source tarball may not contain files outside a directory."); - break; - } - elseif (preg_match('/^[^\/]+\/[^\/]+\//', $tar_file['filename'])) { - $error = __("Error - source tarball may not contain nested subdirectories."); - break; - } } if (!$error && empty($pkgbuild_raw)) { -- 1.7.9.4
participants (3)
-
Dave Reisner
-
Loui Chang
-
Lukas Fleischer