Skip to content

Commit

Permalink
bug #19872 [Filesystem] Consider the umask setting when dumping a fil…
Browse files Browse the repository at this point in the history
…e (leofeyer)

This PR was merged into the 3.1 branch.

Discussion
----------

[Filesystem] Consider the umask setting when dumping a file

| Q             | A
| ------------- | ---
| Branch?       | 3.1
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #14246 , #17063
| License       | MIT
| Doc PR        | -

`Filesystem::dumpFile()` does not consider the umask setting and thus creates files with write permissions for everyone (`0666`). Other `chmod()` calls in Symfony do consider the umask setting (see e.g. [here](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/File/File.php#L101) or [here](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/File/UploadedFile.php#L230)), therefore I have adjusted the `dumpFile()` method accordingly.

Commits
-------

fdd266f Consider the umask setting when dumping a file.
  • Loading branch information
fabpot committed Sep 14, 2016
2 parents c2b660d + fdd266f commit da565de
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
3 changes: 1 addition & 2 deletions src/Symfony/Component/Filesystem/Filesystem.php
Expand Up @@ -556,8 +556,7 @@ public function dumpFile($filename, $content)
throw new IOException(sprintf('Failed to write file "%s".', $filename), 0, null, $filename);
}

// Ignore for filesystems that do not support umask
@chmod($tmpFile, 0666);
@chmod($tmpFile, 0666 & ~umask());
$this->rename($tmpFile, $filename, true);
}

Expand Down
8 changes: 7 additions & 1 deletion src/Symfony/Component/Filesystem/Tests/FilesystemTest.php
Expand Up @@ -1101,13 +1101,19 @@ public function testDumpFile()
{
$filename = $this->workspace.DIRECTORY_SEPARATOR.'foo'.DIRECTORY_SEPARATOR.'baz.txt';

// skip mode check on Windows
if ('\\' !== DIRECTORY_SEPARATOR) {
$oldMask = umask(0002);
}

$this->filesystem->dumpFile($filename, 'bar');
$this->assertFileExists($filename);
$this->assertSame('bar', file_get_contents($filename));

// skip mode check on Windows
if ('\\' !== DIRECTORY_SEPARATOR) {
$this->assertFilePermissions(666, $filename);
$this->assertFilePermissions(664, $filename);
umask($oldMask);
}
}

Expand Down

0 comments on commit da565de

Please sign in to comment.