Skip to content

Commit

Permalink
[5.7] Fix filesystem locking hangs in PackageManifest::build() (larav…
Browse files Browse the repository at this point in the history
…el#25898)

  - Filesystem.php

     1. Created a new `Filesystem::replace()` method that atomically
        replaces a file if it already exists.

  - PackageManifest.php

     1. The Filesystem::get() call in PackageManifest::getManifest() no
        longer has to use an exclusive lock because the packages.php
        manifest file will always be replaced atomically.

     2. Use the new Filesystem::replace() method in
        PackageManifest::write()
  • Loading branch information
Yogarine committed Oct 25, 2018
1 parent 6e3d568 commit 633a907
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 4 deletions.
25 changes: 25 additions & 0 deletions src/Illuminate/Filesystem/Filesystem.php
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,31 @@ public function put($path, $contents, $lock = false)
return file_put_contents($path, $contents, $lock ? LOCK_EX : 0);
}

/**
* Write the contents of a file, replacing it atomically if it already exists.
*
* This will replace the target file permissions. It also resolves symlinks to replace the symlink's target file.
*
* @param string $path
* @param string $content
*/
public function replace($path, $content)
{
// Just in case path already exists and is a symlink, we want to make sure we get the real path.
clearstatcache(true, $path);
$path = realpath($path) ?: $path;

// Write out the contents to a temp file, so we then can rename the file atomically.
$tempPath = tempnam(dirname($path), basename($path));

// Fix permissions of tempPath because `tempnam()` creates it with permissions set to 0600.
chmod($tempPath, 0777 - umask());

file_put_contents($tempPath, $content);

rename($tempPath, $path);
}

/**
* Prepend to a file.
*
Expand Down
7 changes: 3 additions & 4 deletions src/Illuminate/Foundation/PackageManifest.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ protected function getManifest()
$this->build();
}

$this->files->get($this->manifestPath, true);
$this->files->get($this->manifestPath);

return $this->manifest = file_exists($this->manifestPath) ?
$this->files->getRequire($this->manifestPath) : [];
Expand Down Expand Up @@ -168,9 +168,8 @@ protected function write(array $manifest)
throw new Exception('The '.dirname($this->manifestPath).' directory must be present and writable.');
}

$this->files->put(
$this->manifestPath, '<?php return '.var_export($manifest, true).';',
true
$this->files->replace(
$this->manifestPath, '<?php return '.var_export($manifest, true).';'
);
}
}
51 changes: 51 additions & 0 deletions tests/Filesystem/FilesystemTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,45 @@ public function testPutStoresFiles()
$this->assertStringEqualsFile($this->tempDir.'/file.txt', 'Hello World');
}

public function testReplaceStoresFiles()
{
$tempFile = "{$this->tempDir}/file.txt";
$symlinkDir = "{$this->tempDir}/symlink_dir";
$symlink = "{$symlinkDir}/symlink.txt";

mkdir($symlinkDir);
symlink($tempFile, $symlink);

// Prevent changes to symlink_dir
chmod($symlinkDir, 0555);

// Test with a weird non-standard umask.
$umask = 0131;
$originalUmask = umask($umask);

$filesystem = new Filesystem;

// Test replacing non-existent file.
$filesystem->replace($tempFile, 'Hello World');
$this->assertStringEqualsFile($tempFile, 'Hello World');
$this->assertEquals($umask, 0777 - $this->getFilePermissions($tempFile));

// Test replacing existing file.
$filesystem->replace($tempFile, 'Something Else');
$this->assertStringEqualsFile($tempFile, 'Something Else');
$this->assertEquals($umask, 0777 - $this->getFilePermissions($tempFile));

// Test replacing symlinked file.
$filesystem->replace($symlink, 'Yet Something Else Again');
$this->assertStringEqualsFile($tempFile, 'Yet Something Else Again');
$this->assertEquals($umask, 0777 - $this->getFilePermissions($tempFile));

umask($originalUmask);

// Reset changes to symlink_dir
chmod($symlinkDir, 0777 - $originalUmask);
}

public function testSetChmod()
{
file_put_contents($this->tempDir.'/file.txt', 'Hello World');
Expand Down Expand Up @@ -496,4 +535,16 @@ public function testHash()
$filesystem = new Filesystem;
$this->assertEquals('acbd18db4cc2f85cedef654fccc4a4d8', $filesystem->hash($this->tempDir.'/foo.txt'));
}

/**
* @param string $file
* @return int
*/
private function getFilePermissions($file)
{
$filePerms = fileperms($file);
$filePerms = substr(sprintf('%o', $filePerms), -3);

return (int) base_convert($filePerms, 8, 10);
}
}

0 comments on commit 633a907

Please sign in to comment.