Skip to content

Commit

Permalink
MDL-36959 rework adding of content files to the file pool
Browse files Browse the repository at this point in the history 
This patch includes refreshing of borked files in file pool and basic prevention of race conditions. It also helps with diagnosing of file pool permission problems, detects coding errors and some other type of problems including sha1 collision jackpot.
  • Loading branch information
@skodak
skodak committed Apr 20, 2013
1 parent 3a8c438 commit d91e2c1
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 30 deletions.
1 change: 1 addition & 0 deletions lang/en/error.php
View file
Expand Up @@ -472,6 +472,7 @@
$string['statscatchupmode'] = 'Statistics is currently in catchup mode. So far {$a->daysdone} day(s) have been processed and {$a->dayspending} are pending. Check back soon!';
$string['statsdisable'] = 'Statistics are not enabled.';
$string['statsnodata'] = 'There is no available data for that combination of course and time period';
$string['storedfilecannotcreatefile'] = 'Can not create local file pool file, please verify permissions in dataroot and available disk space.';
$string['storedfilecannotcreatefiledirs'] = 'Can not create local file pool directories, please verify permissions in dataroot.';
$string['storedfilecannotread'] = 'Can not read file, either file does not exist or there are permission problems';
$string['storedfilenotcreated'] = 'Can not create file "{$a->contextid}/{$a->component}/{$a->filearea}/{$a->itemid}/{$a->filepath}/{$a->filename}"';
Expand Down
131 changes: 101 additions & 30 deletions lib/filestorage/file_storage.php
View file
Expand Up @@ -1558,40 +1558,90 @@ public function add_file_to_pool($pathname, $contenthash = NULL) {
throw new file_exception('storedfilecannotread', '', $pathname);
}

$filesize = filesize($pathname);
if ($filesize === false) {
throw new file_exception('storedfilecannotread', '', $pathname);
}

if (is_null($contenthash)) {
$contenthash = sha1_file($pathname);
} else if (debugging('', DEBUG_DEVELOPER)) {
$filehash = sha1_file($pathname);
if ($filehash === false) {
throw new file_exception('storedfilecannotread', '', $pathname);
}
if ($filehash !== $contenthash) {
// Hopefully this never happens, if yes we need to fix calling code.
debugging("Invalid contenthash submitted for file $pathname");
$contenthash = $filehash;
}
}
if ($contenthash === false) {
throw new file_exception('storedfilecannotread', '', $pathname);
}

$filesize = filesize($pathname);
if ($filesize > 0 and $contenthash === sha1('')) {
// Did the file change or is sha1_file() borked for this file?
clearstatcache();
$contenthash = sha1_file($pathname);
$filesize = filesize($pathname);

if ($contenthash === false or $filesize === false) {
throw new file_exception('storedfilecannotread', '', $pathname);
}
if ($filesize > 0 and $contenthash === sha1('')) {
// This is very weird...
throw new file_exception('storedfilecannotread', '', $pathname);
}
}

$hashpath = $this->path_from_hash($contenthash);
$hashfile = "$hashpath/$contenthash";

$newfile = true;

if (file_exists($hashfile)) {
if (filesize($hashfile) !== $filesize) {
if (filesize($hashfile) === $filesize) {
return array($contenthash, $filesize, false);
}
if (sha1_file($hashfile) === $contenthash) {
// Jackpot! We have a sha1 collision.
mkdir("$this->filedir/jackpot/", $this->dirpermissions, true);
copy($hashfile, "$this->filedir/jackpot/{$contenthash}_1");
copy($hashfile, "$this->filedir/jackpot/{$contenthash}_2");
throw new file_pool_content_exception($contenthash);
}
debugging("Replacing invalid content file $contenthash");
unlink($hashfile);
$newfile = false;
}

} else {
if (!is_dir($hashpath)) {
if (!mkdir($hashpath, $this->dirpermissions, true)) {
throw new file_exception('storedfilecannotcreatefiledirs'); // permission trouble
}
if (!is_dir($hashpath)) {
if (!mkdir($hashpath, $this->dirpermissions, true)) {
// Permission trouble.
throw new file_exception('storedfilecannotcreatefiledirs');
}
$newfile = true;
}

if (!copy($pathname, $hashfile)) {
throw new file_exception('storedfilecannotread', '', $pathname);
}
// Let's try to prevent some race conditions.

if (filesize($hashfile) !== $filesize) {
@unlink($hashfile);
throw new file_pool_content_exception($contenthash);
}
chmod($hashfile, $this->filepermissions); // fix permissions if needed
$prev = ignore_user_abort(true);
@unlink($hashfile.'.tmp');
if (!copy($pathname, $hashfile.'.tmp')) {
// Borked permissions or out of disk space.
ignore_user_abort($prev);
throw new file_exception('storedfilecannotcreatefile');
}

if (filesize($hashfile.'.tmp') !== $filesize) {
// This should not happen.
unlink($hashfile.'.tmp');
ignore_user_abort($prev);
throw new file_exception('storedfilecannotcreatefile');
}
rename($hashfile.'.tmp', $hashfile);
chmod($hashfile, $this->filepermissions); // Fix permissions if needed.
@unlink($hashfile.'.tmp'); // Just in case anything fails in a weird way.
ignore_user_abort($prev);

return array($contenthash, $filesize, $newfile);
}
Expand All @@ -1609,29 +1659,50 @@ public function add_string_to_pool($content) {
$hashpath = $this->path_from_hash($contenthash);
$hashfile = "$hashpath/$contenthash";

$newfile = true;

if (file_exists($hashfile)) {
if (filesize($hashfile) !== $filesize) {
if (filesize($hashfile) === $filesize) {
return array($contenthash, $filesize, false);
}
if (sha1_file($hashfile) === $contenthash) {
// Jackpot! We have a sha1 collision.
mkdir("$this->filedir/jackpot/", $this->dirpermissions, true);
copy($hashfile, "$this->filedir/jackpot/{$contenthash}_1");
file_put_contents("$this->filedir/jackpot/{$contenthash}_2", $content);
throw new file_pool_content_exception($contenthash);
}
debugging("Replacing invalid content file $contenthash");
unlink($hashfile);
$newfile = false;
}

} else {
if (!is_dir($hashpath)) {
if (!mkdir($hashpath, $this->dirpermissions, true)) {
throw new file_exception('storedfilecannotcreatefiledirs'); // permission trouble
}
if (!is_dir($hashpath)) {
if (!mkdir($hashpath, $this->dirpermissions, true)) {
// Permission trouble.
throw new file_exception('storedfilecannotcreatefiledirs');
}
$newfile = true;
}

file_put_contents($hashfile, $content);
// Hopefully this works around most potential race conditions.

if (filesize($hashfile) !== $filesize) {
@unlink($hashfile);
throw new file_pool_content_exception($contenthash);
}
chmod($hashfile, $this->filepermissions); // fix permissions if needed
$prev = ignore_user_abort(true);
$newsize = file_put_contents($hashfile.'.tmp', $content, LOCK_EX);
if ($newsize === false) {
// Borked permissions most likely.
ignore_user_abort($prev);
throw new file_exception('storedfilecannotcreatefile');
}
if (filesize($hashfile.'.tmp') !== $filesize) {
// Out of disk space?
unlink($hashfile.'.tmp');
ignore_user_abort($prev);
throw new file_exception('storedfilecannotcreatefile');
}
rename($hashfile.'.tmp', $hashfile);
chmod($hashfile, $this->filepermissions); // Fix permissions if needed.
@unlink($hashfile.'.tmp'); // Just in case anything fails in a weird way.
ignore_user_abort($prev);

return array($contenthash, $filesize, $newfile);
}
Expand Down

0 comments on commit d91e2c1

Please sign in to comment.