Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BUGFIX: Avoid race condition on symlink publishing #2669

Merged
merged 8 commits into from Mar 23, 2022
Merged

BUGFIX: Avoid race condition on symlink publishing #2669

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bb7279d
TASK: Extract duplicate publish code into method
kdambekalns Jan 19, 2022
69ed476
BUGFIX: Check for correct symlink after error
kdambekalns Jan 19, 2022
404dc0e
Merge branch '6.3' into bugfix/2667-publish-symlink-race-condition
kdambekalns Mar 10, 2022
eb2f8b2
Merge branch '6.3' into bugfix/2667-publish-symlink-race-condition
kdambekalns Mar 15, 2022
1004aa1
Merge branch '6.3' into bugfix/2667-publish-symlink-race-condition
kdambekalns Mar 15, 2022
6e464c2
Merge branch '6.3' into bugfix/2667-publish-symlink-race-condition
kdambekalns Mar 18, 2022
f824cab
Merge branch '6.3' into bugfix/2667-publish-symlink-race-condition
kdambekalns Mar 22, 2022
bbf39f2
Merge branch '6.3' into bugfix/2667-publish-symlink-race-condition
jonnitto Mar 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
77 changes: 37 additions & 40 deletions Neos.Flow/Classes/ResourceManagement/Target/FileSystemSymlinkTarget.php
View file
Open in desktop
Expand Up @@ -76,27 +76,9 @@ protected function publishFile($sourceStream, $relativeTargetPathAndFilename)
throw new TargetException(sprintf('Could not publish "%s" into resource publishing target "%s" because the source file is not accessible (file stat failed).', $sourcePathAndFilename, $this->name), 1415716366);
}

if (!file_exists(dirname($targetPathAndFilename))) {
Files::createDirectoryRecursively(dirname($targetPathAndFilename));
}

try {
if (Files::is_link($targetPathAndFilename)) {
Files::unlink($targetPathAndFilename);
}

if ($this->relativeSymlinks) {
$result = Files::createRelativeSymlink($sourcePathAndFilename, $targetPathAndFilename);
} else {
$temporaryTargetPathAndFilename = $targetPathAndFilename . '.' . Algorithms::generateRandomString(13) . '.tmp';
symlink($sourcePathAndFilename, $temporaryTargetPathAndFilename);
$result = rename($temporaryTargetPathAndFilename, $targetPathAndFilename);
}
} catch (\Exception $exception) {
$result = false;
}
[$result, $exception] = $this->publish($targetPathAndFilename, $sourcePathAndFilename);
if ($result === false) {
throw new TargetException(sprintf('Could not publish "%s" into resource publishing target "%s" because the source file could not be symlinked at target location.', $sourcePathAndFilename, $this->name), 1415716368, (isset($exception) ? $exception : null));
throw new TargetException(sprintf('Could not publish "%s" into resource publishing target "%s" because the source file could not be symlinked at target location.', $sourcePathAndFilename, $this->name), 1415716368, ($exception ?? null));
}

$this->logger->debug(sprintf('FileSystemSymlinkTarget: Published file. (target: %s, file: %s)', $this->name, $relativeTargetPathAndFilename));
Expand Down Expand Up @@ -131,8 +113,8 @@ protected function unpublishFile($relativeTargetPathAndFilename)
*
* @param string $sourcePath Absolute path to the source directory
* @param string $relativeTargetPathAndFilename relative path and filename in the target directory
* @throws TargetException
* @return void
* @throws TargetException
*/
protected function publishDirectory($sourcePath, $relativeTargetPathAndFilename)
{
Expand All @@ -142,26 +124,9 @@ protected function publishDirectory($sourcePath, $relativeTargetPathAndFilename)
throw new TargetException(sprintf('Could not publish directory "%s" into resource publishing target "%s" because the source is not accessible (file stat failed).', $sourcePath, $this->name), 1416244512);
}

if (!file_exists(dirname($targetPathAndFilename))) {
Files::createDirectoryRecursively(dirname($targetPathAndFilename));
}

try {
if (Files::is_link($targetPathAndFilename)) {
Files::unlink($targetPathAndFilename);
}
if ($this->relativeSymlinks) {
$result = Files::createRelativeSymlink($sourcePath, $targetPathAndFilename);
} else {
$temporaryTargetPathAndFilename = $targetPathAndFilename . '.' . Algorithms::generateRandomString(13) . '.tmp';
symlink($sourcePath, $temporaryTargetPathAndFilename);
$result = rename($temporaryTargetPathAndFilename, $targetPathAndFilename);
}
} catch (\Exception $exception) {
$result = false;
}
[$result, $exception] = $this->publish($targetPathAndFilename, $sourcePath);
if ($result === false) {
throw new TargetException(sprintf('Could not publish "%s" into resource publishing target "%s" because the source directory could not be symlinked at target location.', $sourcePath, $this->name), 1416244515, (isset($exception) ? $exception : null));
throw new TargetException(sprintf('Could not publish "%s" into resource publishing target "%s" because the source directory could not be symlinked at target location.', $sourcePath, $this->name), 1416244515, ($exception ?? null));
}

$this->logger->debug(sprintf('FileSystemSymlinkTarget: Published directory. (target: %s, file: %s)', $this->name, $relativeTargetPathAndFilename));
Expand All @@ -183,4 +148,36 @@ protected function setOption($key, $value)

return parent::setOption($key, $value);
}

private function publish(string $targetPathAndFilename, string $sourcePathAndFilename): array
{
$exception = null;

if (!file_exists(dirname($targetPathAndFilename))) {
Files::createDirectoryRecursively(dirname($targetPathAndFilename));
}

try {
if (Files::is_link($targetPathAndFilename)) {
Files::unlink($targetPathAndFilename);
}

if ($this->relativeSymlinks) {
$result = Files::createRelativeSymlink($sourcePathAndFilename, $targetPathAndFilename);
} else {
$temporaryTargetPathAndFilename = $targetPathAndFilename . '.' . Algorithms::generateRandomString(13) . '.tmp';
symlink($sourcePathAndFilename, $temporaryTargetPathAndFilename);
$result = rename($temporaryTargetPathAndFilename, $targetPathAndFilename);
}
} catch (\Exception $exception) {
$result = false;
}

if (Files::is_link($targetPathAndFilename) && realpath($targetPathAndFilename) === realpath($sourcePathAndFilename)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can there be a situation where both realpath return false? (since false === false)
i only know that realpath doesnt like stream wrapper and returns false then ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, well. Technically, yes, that could happen. But I think we can ignore that here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

$this->logger->debug(sprintf('FileSystemSymlinkTarget: File already published, probably a concurrent write. (target: %s, file: %s)', $this->name, $targetPathAndFilename));
return [true, null];
}

return [$result, $exception];
}
}