Arbit - project tracking

Dwoo

#18: *nix: invalid permissions on created dirs

Issue attachments

Issue revisions

  • closed by Mikhail Krasilnikov at 2009-J-30 11:25
  • closed by Mikhail Krasilnikov at 2009-J-30 11:25
Type bug bug
State closed closed
Priority normal normal
Resolution fixed fixed
Assigned to Jordi Boggiano
Scheduled for 1.1.0
Affected versions
Affected components Core
Last change Friday 30 January 2009 11:25:15 UTC by Mikhail Krasilnikov

String::makeDirectory sets aside system umask value. So in case when script's owner is 'www' (for example) and umask is 0022 (default for most Linux systems) and mkdir('xxx', 0777, true), permissions will be: <pre> Access: (0755/drwxr-xr-x) Uid: ( 30/ www) Gid: ( 8/ www) </pre> And no one except www and root can delete this directories.

This can be avoided by temporary setting umask to '0000' (see attached patch).

But there is a note in PHP manual (http://php.net/manual/en/function.umask.php): "Note: Avoid using this function in multithreaded webservers. It is better to change the file permissions with chmod() after creating the file. Using umask() can lead to unexpected behavior of concurrently running scripts and the webserver itself because they all use the same umask. "

  • Jordi Boggiano at Wednesday 4 February 2009 21:07:39 UTC

    I'm not entirely sure what's the issue there.. Is it a bad thing that the directory isn't writable by anyone else than root/www?

  • Mikhail Krasilnikov at Thursday 5 February 2009 06:52:56 UTC

    Jordi Boggiano wrote: > I'm not entirely sure what's the issue there.. Is it a bad thing that the directory isn't writable by anyone else than root/www?

    In case of virtual hosting - yes. Most of virtual hosting users have only FTP access. And FTP user is different from root/www. So these users can't delete dirs created by Dwoo.

    In my case it's not a virtual hosting, and we have a root access, but we using automated upload to server via FTP with cleaning up all remote files before upload. Cleaning up fails when trying to remove cache dirs.

    And in whole I think it's not right to deprive user (site owner) of opportunity to remove any file he want.

  • Jordi Boggiano at Thursday 5 February 2009 08:50:30 UTC

    Okay got it now, I wasn't aware mkdir() mode was affected by the umask.. Anyway, I can do a chmod on the folder afterwards, that's no big deal, one problem remains though.. I use mkdir's recursive mode, and if I only apply the chmod to the entire path, intermediate folders won't have the right mode set. So I guess that means recursive chmod'ing, yay. Or maybe I can do : <pre><code class="php"> exec('chmod -R '.$this->chmod.' '.$this->getCacheDir().'/'...blah)</code></pre>

    What do you think ? I'm not too aware of shared hosting restrictions as to exec()'s usage.

  • Mikhail Krasilnikov at Thursday 5 February 2009 09:39:26 UTC

    Advice from PHP devs is to set umask ([[http://bugs.php.net/bug.php?id=37027]])

    <pre> mkdir(dir, mode) actualy sets permissions to (mode & ~umask & 0777). So you have to use umask() to

    1. unset the correct umask;

    2. create the directory with mode 0777;

    3) reset umask to it's old value. </pre>

    So I think patch in my first message can be used.

  • Jordi Boggiano at Thursday 5 February 2009 10:07:19 UTC

    Okay.. I'll see, but the potential race condition with umask() isn't that great, although it's not too likely it will happen since compilation is a fairly rare event.

  • Mikhail Krasilnikov at Thursday 5 February 2009 10:40:33 UTC

    Jordi Boggiano wrote:
> but the potential race condition with umask() isn't that great

I agree... But "exec" is also not too good solution. May be split path by DIRECTORY_SEPARATOR and use PHP's chmod in cycle? Something like that:
<pre>
$dirs = explode(DIRECTORY_SEPARATOR, $path);
$tmpPath = 'cache/root/dir';
foreach ($dirs as $dir) {
  $tmpPath .= DIRECTORY_SEPARATOR . $dir;
  chmod($tmpPath, $mode);
}
</pre>

  • Jordi Boggiano at Thursday 5 February 2009 13:34:45 UTC

    Applied in changeset r244.

  • Mikhail Krasilnikov at Thursday 5 February 2009 13:50:02 UTC

    Thanks!

  • Jordi Boggiano at Thursday 5 February 2009 13:55:09 UTC

    No problem, hope it is really solved.. I kinda tested it on windows but erm, let me know if it fails.

  • Mikhail Krasilnikov at Thursday 5 February 2009 14:52:24 UTC

    Yep. I've got an error:
<pre>
Description: chmod(): Operation not permitted
Backtrace/context:
Array
(
    [path] => /home/mekras/public_html/vhosts/russkieskidki.local/root/templates/partner
    [baseDir] => /home/mekras/public_html/vhosts/russkieskidki.local/root/core/templates/compiled//
    [chmod] => 511
    [folders] => Array
        (
            [0] =>
            [1] => home
            [2] => mekras
            [3] => public_html
            [4] => vhosts
            [5] => russkieskidki.local
            [6] => root
            [7] => templates
            [8] => partner
        )

    [folder] =>
)
</pre>

Dwoo engine located at /home/mekras/public_html/vhosts/russkieskidki.local/root/core/templates/

I think that it's because $path argument have a strange value:

"/home/mekras/public_html/vhosts/russkieskidki.local/root/core/templates/compiled//home/mekras/public_html/vhosts/russkieskidki.local/root/templates/partner"

which constructed from compileDir (/home/mekras/public_html/vhosts/russkieskidki.local/root/core/templates/compiled/) and ResourceIdentifier (/home/mekras/public_html/vhosts/russkieskidki.local/root/templates/partner)

  • Mikhail Krasilnikov at Thursday 5 February 2009 14:54:17 UTC

    Simple workaround is modify String.php:479: <pre> foreach ($folders as $folder) if ($folder !== '') {

    </pre>

  • Jordi Boggiano at Thursday 5 February 2009 15:34:20 UTC

    Applied in changeset r245.

Arbit - project tracking, licensed under GPL 3