- Affected version
- 2.3.6
When importing a style archive, XenForo may copy files over existing files.
This operation is not atomic and thus those files may be accessed by a client in an inconsistent state (eg. half-written).
XF\Service\Style\ArchiveImport::copyAssetFiles
While there most likely isn't much XenForo could do to prevent this when using Flysystem, IMHO XenForo should try to avoid this when directly writing to the filesystem, eg. when importing via CLI.
XF\Util\File::copyFile
Suggested Workaround
Copy to a new temporary file in the same directory first and afterwards rename onto target; if rename fails delete target and try rename again.
This still isn't fully atomic (on Windows) so could cause a brief 404 but should be better than half-written files.
This operation is not atomic and thus those files may be accessed by a client in an inconsistent state (eg. half-written).
XF\Service\Style\ArchiveImport::copyAssetFiles
Code:
if ($this->rewriteAssetPaths)
{
File::copyFileToAbstractedPath($pathname, $dataUriPrefix . $stdPath);
}
else
{
$newPathPrefix = \XF::getRootDirectory() . $DS;
File::copyFile($pathname, $newPathPrefix . $stdPath);
}
While there most likely isn't much XenForo could do to prevent this when using Flysystem, IMHO XenForo should try to avoid this when directly writing to the filesystem, eg. when importing via CLI.
XF\Util\File::copyFile
PHP:
public static function copyFile($source, $destination, $createIndexHtml = true)
{
$dir = dirname($destination);
if (static::createDirectory($dir, $createIndexHtml))
{
if (copy($source, $destination))
{
static::makeWritableByFtpUser($destination);
return true;
}
else
{
return false;
}
}
else
{
return false;
}
}
Suggested Workaround
Copy to a new temporary file in the same directory first and afterwards rename onto target; if rename fails delete target and try rename again.
This still isn't fully atomic (on Windows) so could cause a brief 404 but should be better than half-written files.