diff --git a/lang/en/error.php b/lang/en/error.php index 66fc80a0da3..d98425b8645 100644 --- a/lang/en/error.php +++ b/lang/en/error.php @@ -472,6 +472,7 @@ $string['sslonlyaccess'] = 'For security reasons only https connections are allo $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}"'; diff --git a/lib/filestorage/file_storage.php b/lib/filestorage/file_storage.php index ee29b75f6a1..e1dd7d6bedd 100644 --- a/lib/filestorage/file_storage.php +++ b/lib/filestorage/file_storage.php @@ -1558,40 +1558,90 @@ class file_storage { throw new file_exception('storedfilecannotread', '', $pathname); } - if (is_null($contenthash)) { - $contenthash = sha1_file($pathname); + $filesize = filesize($pathname); + if ($filesize === false) { + throw new file_exception('storedfilecannotread', '', $pathname); } - $filesize = filesize($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); + } + + 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 - } - } - $newfile = true; - - if (!copy($pathname, $hashfile)) { - throw new file_exception('storedfilecannotread', '', $pathname); - } - - if (filesize($hashfile) !== $filesize) { - @unlink($hashfile); - throw new file_pool_content_exception($contenthash); - } - chmod($hashfile, $this->filepermissions); // fix permissions if needed } + if (!is_dir($hashpath)) { + if (!mkdir($hashpath, $this->dirpermissions, true)) { + // Permission trouble. + throw new file_exception('storedfilecannotcreatefiledirs'); + } + } + + // Let's try to prevent some race conditions. + + $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); } @@ -1609,30 +1659,51 @@ class file_storage { $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 - } - } - $newfile = true; - - file_put_contents($hashfile, $content); - - if (filesize($hashfile) !== $filesize) { - @unlink($hashfile); - throw new file_pool_content_exception($contenthash); - } - chmod($hashfile, $this->filepermissions); // fix permissions if needed } + if (!is_dir($hashpath)) { + if (!mkdir($hashpath, $this->dirpermissions, true)) { + // Permission trouble. + throw new file_exception('storedfilecannotcreatefiledirs'); + } + } + + // Hopefully this works around most potential race conditions. + + $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); } diff --git a/lib/filestorage/tests/file_storage_test.php b/lib/filestorage/tests/file_storage_test.php index b895e578bb5..709e886fc82 100644 --- a/lib/filestorage/tests/file_storage_test.php +++ b/lib/filestorage/tests/file_storage_test.php @@ -29,13 +29,168 @@ defined('MOODLE_INTERNAL') || die(); global $CFG; require_once($CFG->libdir . '/filelib.php'); require_once($CFG->dirroot . '/repository/lib.php'); +require_once($CFG->libdir . '/filestorage/stored_file.php'); class filestoragelib_testcase extends advanced_testcase { + /** + * Files can be created from strings. + */ + public function test_create_file_from_string() { + global $DB; + + $this->resetAfterTest(true); + + $this->assertEquals(0, $DB->count_records('files', array())); + + $content = 'abcd'; + $syscontext = context_system::instance(); + $filerecord = array( + 'contextid' => $syscontext->id, + 'component' => 'core', + 'filearea' => 'unittest', + 'itemid' => 0, + 'filepath' => '/images/', + 'filename' => 'testfile.txt', + ); + $pathhash = sha1('/'.$filerecord['contextid'].'/'.$filerecord['component'].'/'.$filerecord['filearea'].'/'.$filerecord['itemid'].$filerecord['filepath'].$filerecord['filename']); + + $fs = get_file_storage(); + $file = $fs->create_file_from_string($filerecord, $content); + + $this->assertInstanceOf('stored_file', $file); + $this->assertSame(sha1($content), $file->get_contenthash()); + $this->assertSame($pathhash, $file->get_pathnamehash()); + + $this->assertTrue($DB->record_exists('files', array('pathnamehash'=>$pathhash))); + + $location = test_stored_file_inspection::get_pretected_pathname($file); + + $this->assertFileExists($location); + + + // Verify the dir placeholder files are created. + $this->assertEquals(3, $DB->count_records('files', array())); + $this->assertTrue($DB->record_exists('files', array('pathnamehash'=>sha1('/'.$filerecord['contextid'].'/'.$filerecord['component'].'/'.$filerecord['filearea'].'/'.$filerecord['itemid'].'/.')))); + $this->assertTrue($DB->record_exists('files', array('pathnamehash'=>sha1('/'.$filerecord['contextid'].'/'.$filerecord['component'].'/'.$filerecord['filearea'].'/'.$filerecord['itemid'].$filerecord['filepath'].'.')))); + + + // Tests that missing content file is recreated. + + unlink($location); + $this->assertFileNotExists($location); + + $filerecord['filename'] = 'testfile2.txt'; + $file2 = $fs->create_file_from_string($filerecord, $content); + $this->assertInstanceOf('stored_file', $file2); + $this->assertSame($file->get_contenthash(), $file2->get_contenthash()); + $this->assertFileExists($location); + + $this->assertEquals(4, $DB->count_records('files', array())); + + + // Test that borked content file is recreated. + + $this->assertSame(2, file_put_contents($location, 'xx')); + + $filerecord['filename'] = 'testfile3.txt'; + $file3 = $fs->create_file_from_string($filerecord, $content); + $this->assertInstanceOf('stored_file', $file3); + $this->assertSame($file->get_contenthash(), $file3->get_contenthash()); + $this->assertFileExists($location); + + $this->assertSame($content, file_get_contents($location)); + $this->assertDebuggingCalled(); + + $this->assertEquals(5, $DB->count_records('files', array())); + } + /** * Local files can be added to the filepool */ public function test_create_file_from_pathname() { + global $CFG, $DB; + + $this->resetAfterTest(true); + + $filecount = $DB->count_records('files', array()); + $this->assertEquals(0, $filecount); + + $filepath = $CFG->dirroot.'/lib/filestorage/tests/fixtures/testimage.jpg'; + $syscontext = context_system::instance(); + $filerecord = array( + 'contextid' => $syscontext->id, + 'component' => 'core', + 'filearea' => 'unittest', + 'itemid' => 0, + 'filepath' => '/images/', + 'filename' => 'testimage.jpg', + ); + $pathhash = sha1('/'.$filerecord['contextid'].'/'.$filerecord['component'].'/'.$filerecord['filearea'].'/'.$filerecord['itemid'].$filerecord['filepath'].$filerecord['filename']); + + $fs = get_file_storage(); + $file = $fs->create_file_from_pathname($filerecord, $filepath); + + $this->assertInstanceOf('stored_file', $file); + $this->assertSame(sha1_file($filepath), $file->get_contenthash()); + + $this->assertTrue($DB->record_exists('files', array('pathnamehash'=>$pathhash))); + + $location = test_stored_file_inspection::get_pretected_pathname($file); + + $this->assertFileExists($location); + + + // Verify the dir placeholder files are created. + $this->assertEquals(3, $DB->count_records('files', array())); + $this->assertTrue($DB->record_exists('files', array('pathnamehash'=>sha1('/'.$filerecord['contextid'].'/'.$filerecord['component'].'/'.$filerecord['filearea'].'/'.$filerecord['itemid'].'/.')))); + $this->assertTrue($DB->record_exists('files', array('pathnamehash'=>sha1('/'.$filerecord['contextid'].'/'.$filerecord['component'].'/'.$filerecord['filearea'].'/'.$filerecord['itemid'].$filerecord['filepath'].'.')))); + + + // Tests that missing content file is recreated. + + unlink($location); + $this->assertFileNotExists($location); + + $filerecord['filename'] = 'testfile2.jpg'; + $file2 = $fs->create_file_from_pathname($filerecord, $filepath); + $this->assertInstanceOf('stored_file', $file2); + $this->assertSame($file->get_contenthash(), $file2->get_contenthash()); + $this->assertFileExists($location); + + $this->assertEquals(4, $DB->count_records('files', array())); + + + // Test that borked content file is recreated. + + $this->assertSame(2, file_put_contents($location, 'xx')); + + $filerecord['filename'] = 'testfile3.jpg'; + $file3 = $fs->create_file_from_pathname($filerecord, $filepath); + $this->assertInstanceOf('stored_file', $file3); + $this->assertSame($file->get_contenthash(), $file3->get_contenthash()); + $this->assertFileExists($location); + + $this->assertSame(file_get_contents($filepath), file_get_contents($location)); + $this->assertDebuggingCalled(); + + $this->assertEquals(5, $DB->count_records('files', array())); + + // Test invalid file creation. + + $filerecord['filename'] = 'testfile4.jpg'; + try { + $fs->create_file_from_pathname($filerecord, $filepath.'nonexistent'); + $this->fail('Exception expected when trying to add non-existent stored file.'); + } catch (Exception $e) { + $this->assertInstanceOf('file_exception', $e); + } + } + + /** + * Tests get get file. + */ + public function test_get_file() { global $CFG; $this->resetAfterTest(false); @@ -50,19 +205,28 @@ class filestoragelib_testcase extends advanced_testcase { 'filepath' => '/images/', 'filename' => 'testimage.jpg', ); + $pathhash = sha1('/'.$filerecord['contextid'].'/'.$filerecord['component'].'/'.$filerecord['filearea'].'/'.$filerecord['itemid'].$filerecord['filepath'].$filerecord['filename']); $fs = get_file_storage(); - $fs->create_file_from_pathname($filerecord, $filepath); + $file = $fs->create_file_from_pathname($filerecord, $filepath); - $this->assertTrue($fs->file_exists($syscontext->id, 'core', 'unittest', 0, '/images/', 'testimage.jpg')); + $this->assertInstanceOf('stored_file', $file); + $this->assertEquals($syscontext->id, $file->get_contextid()); + $this->assertEquals('core', $file->get_component()); + $this->assertEquals('unittest', $file->get_filearea()); + $this->assertEquals(0, $file->get_itemid()); + $this->assertEquals('/images/', $file->get_filepath()); + $this->assertEquals('testimage.jpg', $file->get_filename()); + $this->assertEquals(filesize($filepath), $file->get_filesize()); + $this->assertEquals($pathhash, $file->get_pathnamehash()); - return $fs->get_file($syscontext->id, 'core', 'unittest', 0, '/images/', 'testimage.jpg'); + return $file; } /** * Local images can be added to the filepool and their preview can be obtained * - * @depends test_create_file_from_pathname + * @depends test_get_file */ public function test_get_file_preview(stored_file $file) { global $CFG; @@ -1421,5 +1585,10 @@ class filestoragelib_testcase extends advanced_testcase { $this->setExpectedException('coding_exception'); $fs->get_unused_filename($contextid, $component, $filearea, $itemid, $filepath, ''); } - +} + +class test_stored_file_inspection extends stored_file { + public static function get_pretected_pathname(stored_file $file) { + return $file->get_pathname_by_contenthash(); + } }