diff --git a/library/HTMLPurifier/DefinitionCache/Serializer.php b/library/HTMLPurifier/DefinitionCache/Serializer.php index 6ba9ad2d9..aa8615e03 100644 --- a/library/HTMLPurifier/DefinitionCache/Serializer.php +++ b/library/HTMLPurifier/DefinitionCache/Serializer.php @@ -83,7 +83,7 @@ public function remove($config) if (!file_exists($file)) { return false; } - return unlink($file); + return $this->safeUnlink($file); } /** @@ -110,7 +110,7 @@ public function flush($config) if ($filename[0] === '.') { continue; } - unlink($dir . '/' . $filename); + $this->safeUnlink($dir . '/' . $filename); } closedir($dh); return true; @@ -141,7 +141,7 @@ public function cleanup($config) $key = substr($filename, 0, strlen($filename) - 4); $file = $dir . '/' . $filename; if ($this->isOld($key, $config) && file_exists($file)) { - unlink($file); + $this->safeUnlink($file); } } closedir($dh); @@ -188,6 +188,31 @@ public function generateBaseDirectoryPath($config) return $base; } + /** + * Silently handles files already deleted by another process, + * but still emits a warning when the file remains after unlink. + * @param string $file + * @return bool + */ + private function safeUnlink($file) + { + if (!file_exists($file)) { + return true; + } + if (@unlink($file)) { + return true; + } + clearstatcache(true, $file); + if (!file_exists($file)) { + return true; + } + trigger_error( + 'Could not delete definition cache file ' . $file, + E_USER_WARNING + ); + return false; + } + /** * Convenience wrapper function for file_put_contents * @param string $file File name to write to diff --git a/tests/HTMLPurifier/DefinitionCache/SerializerTest.php b/tests/HTMLPurifier/DefinitionCache/SerializerTest.php index 57c2c3e28..b8aba9072 100644 --- a/tests/HTMLPurifier/DefinitionCache/SerializerTest.php +++ b/tests/HTMLPurifier/DefinitionCache/SerializerTest.php @@ -167,6 +167,31 @@ public function testCleanupOnlySameID() $cache->flush($config1); } + public function testSafeUnlinkTreatsMissingFileAsSuccess() + { + $cache = new HTMLPurifier_DefinitionCache_Serializer('Test'); + + $this->assertTrue( + $this->invokeSafeUnlink( + $cache, + dirname(__FILE__) . '/SerializerTest/missing-file-' . uniqid('', true) + ) + ); + } + + public function testSafeUnlinkWarnsWhenTargetStillExists() + { + $cache = new HTMLPurifier_DefinitionCache_Serializer('Test'); + + $dir = dirname(__FILE__) . '/SerializerTestDir-' . uniqid('', true); + mkdir($dir); + + $this->expectError('Could not delete definition cache file ' . $dir); + $this->assertFalse($this->invokeSafeUnlink($cache, $dir)); + + rmdir($dir); + } + /** * Asserts that a file exists, ignoring the stat cache */ @@ -238,6 +263,15 @@ public function testNoInfiniteLoop() $cache->cleanup($config); } + + private function invokeSafeUnlink($cache, $file) + { + $callable = Closure::bind(function ($file) { + return $this->safeUnlink($file); + }, $cache, 'HTMLPurifier_DefinitionCache_Serializer'); + + return $callable($file); + } } // vim: et sw=4 sts=4