fix: handle definition cache delete races safely

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

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