[aur-dev] [PATCH] Revert "Use system rm in rm_rf function."
This reverts commit 5c3f01909301f641f57b2ffe8b59609de6be2256. Falling back to a system function is really dumb when all we need to do is fix the ordering of things in this function. Signed-off-by: Dan McGee <dan@archlinux.org> --- Guys, I sent the last patches based off of the master branch- looks like I have no idea how your development works, sorry about that. Anyway here is the series based off of the testing branch. It still will need some testing. -Dan web/lib/aur.inc | 15 ++++++++++++--- 1 files changed, 12 insertions(+), 3 deletions(-) diff --git a/web/lib/aur.inc b/web/lib/aur.inc index a126bb9..ade5b82 100644 --- a/web/lib/aur.inc +++ b/web/lib/aur.inc @@ -374,10 +374,19 @@ function can_submit_pkg($name="", $sid="") { # recursive delete directory # function rm_rf($dirname="") { - if ($dirname != "") { - exec('rm -rf ' . escapeshellcmd($dirname)); + $d = dir($dirname); + while ($f = $d->read()) { + if ($f != "." && $f != "..") { + if (is_dir($dirname."/".$f)) { + rm_rf($dirname."/".$f); + } + if (is_file($dirname."/".$f) || is_link($dirname."/".$f)) { + unlink($dirname."/".$f); + } + } } - + $d->close(); + rmdir($dirname); return; } -- 1.6.0.3
Fix FS#11187 *correctly*. Signed-off-by: Dan McGee <dan@archlinux.org> --- web/lib/aur.inc | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/web/lib/aur.inc b/web/lib/aur.inc index ade5b82..d0c2bc2 100644 --- a/web/lib/aur.inc +++ b/web/lib/aur.inc @@ -377,11 +377,11 @@ function rm_rf($dirname="") { $d = dir($dirname); while ($f = $d->read()) { if ($f != "." && $f != "..") { - if (is_dir($dirname."/".$f)) { - rm_rf($dirname."/".$f); - } - if (is_file($dirname."/".$f) || is_link($dirname."/".$f)) { - unlink($dirname."/".$f); + $fullpath = $dirname."/".$f; + if (is_file($fullpath) || is_link($fullpath)) { + unlink($fullpath); + } elseif (is_dir($fullpath)) { + rm_rf($fullpath); } } } -- 1.6.0.3
This will chmod a given directory and all its contents to the correct group-writeable permissions. Signed-off-by: Dan McGee <dan@archlinux.org> --- web/lib/aur.inc | 27 +++++++++++++++++++++++++++ 1 files changed, 27 insertions(+), 0 deletions(-) diff --git a/web/lib/aur.inc b/web/lib/aur.inc index d0c2bc2..3bc3637 100644 --- a/web/lib/aur.inc +++ b/web/lib/aur.inc @@ -390,6 +390,33 @@ function rm_rf($dirname="") { return; } +# recursive chmod to set group write permissions +# +function chmod_group($path) { + if (!is_dir($path)) + return chmod($path, 0664); + + $d = dir($path); + while ($f = $d->read()) { + if ($f != '.' && $f != '..') { + $fullpath = $path.'/'.$f; + if (is_link($fullpath)) + continue; + elseif (!is_dir($fullpath)) + if (!chmod($fullpath, 0664)) + return FALSE; + elseif(!chmod_group($fullpath)) + return FALSE; + } + } + $d->close(); + + if(chmod($path, 0775)) + return TRUE; + else + return FALSE; +} + # obtain the uid given a Users.Username # function uid_from_username($username="") -- 1.6.0.3
Use the new chmod_group() function to do so. Signed-off-by: Dan McGee <dan@archlinux.org> --- web/html/pkgsubmit.php | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/web/html/pkgsubmit.php b/web/html/pkgsubmit.php index 78d6e50..3612403 100644 --- a/web/html/pkgsubmit.php +++ b/web/html/pkgsubmit.php @@ -44,6 +44,10 @@ if ($_COOKIE["AURSID"]): if (!$extract) { $error = __("Unknown file format for uploaded file."); } + if (!chmod_group($tempdir)) { + $error = __("Could not chmod directory %s.", + array($tempdir)); + } } } } @@ -215,6 +219,11 @@ if ($_COOKIE["AURSID"]): if (!@mkdir(INCOMING_DIR . $pkg_name)) { $error = __( "Could not create directory %s.", INCOMING_DIR . $pkg_name); + } else { + if (!@chmod(INCOMING_DIR . $pkg_name, 0775)) { + $error = __( "Could not chmod directory %s.", + INCOMING_DIR . $pkg_name); + } } rename($pkg_dir, INCOMING_DIR . $pkg_name . "/" . $pkg_name); -- 1.6.0.3
On Fri, Oct 24, 2008 at 04:29:01PM -0500, Dan McGee wrote:
This reverts commit 5c3f01909301f641f57b2ffe8b59609de6be2256. Falling back to a system function is really dumb when all we need to do is fix the ordering of things in this function.
Falling back on a system function can be dumb, but it seems that PHP unlink() is unable to handle self-referencing symlinks. So this what I optioned to do. I tried to look for another way to no avail. Am I doing something wrong perhaps?
I sent the last patches based off of the master branch- looks like I have no idea how your development works, sorry about that. Anyway here is the series based off of the testing branch. It still will need some testing.
Haha yeah that's mentioned in HACKING on the testing branch. Doh.
On Fri, Oct 24, 2008 at 6:46 PM, Loui <louipc.ist@gmail.com> wrote:
On Fri, Oct 24, 2008 at 04:29:01PM -0500, Dan McGee wrote:
This reverts commit 5c3f01909301f641f57b2ffe8b59609de6be2256. Falling back to a system function is really dumb when all we need to do is fix the ordering of things in this function.
Falling back on a system function can be dumb, but it seems that PHP unlink() is unable to handle self-referencing symlinks. So this what I optioned to do. I tried to look for another way to no avail.
Am I doing something wrong perhaps?
Looks like it is due to PHP security restrictions, not self-referencing symlinks. If you create a symlink to a bogus location, PHP is unable to delete it because of the default open_basedir restructions. ln -s testfile testlink php -r 'unlink("testlink");' The file still exists. So that means one of two things: 1. Remove any open_basedir restrictions 2. Just shell out to do it since we have that command enabled (or do we?)
I sent the last patches based off of the master branch- looks like I have no idea how your development works, sorry about that. Anyway here is the series based off of the testing branch. It still will need some testing.
Haha yeah that's mentioned in HACKING on the testing branch. Doh.
Just a FYI- pretty much every other project I've hacked on using GIT uses master as their location of the current work. :) -Dan
On Fri, Oct 24, 2008 at 08:04:19PM -0500, Dan McGee wrote:
Just a FYI- pretty much every other project I've hacked on using GIT uses master as their location of the current work. :)
Yeah that does make a lot of sense. I guess we're living in the shadow of our predecessors and didn't think twice of it. I've merged testing into master now, and there's a stable branch for what master used to be.
participants (3)
-
Dan McGee
-
Dan McGee
-
Loui