fix: filecache race condition (#637)

Author: bshaffer

composer.json

@@ -26,7 +26,8 @@
     "phpseclib/phpseclib": "^3.0.35",
     "kelvinmo/simplejwt": "0.7.1",
     "webmozart/assert": "^1.11",
-    "symfony/process": "^6.0||^7.0"
+    "symfony/process": "^6.0||^7.0",
+    "symfony/filesystem": "^6.3||^7.3"
   },
   "suggest": {
     "phpseclib/phpseclib": "May be used in place of OpenSSL for signing strings or for token management. Please require version ^2."

src/Cache/FileSystemCacheItemPool.php

@@ -46,7 +46,9 @@ public function __construct(string $path)
             return;
         }
 
-        if (!mkdir($this->cachePath)) {
+        // Suppress the error for when the directory already exists because of a
+        // race condition
+        if (!@mkdir($this->cachePath, 0777, true) && !is_dir($this->cachePath)) {
             throw new ErrorException("Cache folder couldn't be created.");
         }
     }
@@ -111,7 +113,7 @@ public function save(CacheItemInterface $item): bool
         $itemPath = $this->cacheFilePath($item->getKey());
         $serializedItem = serialize($item->get());
 
-        $result = file_put_contents($itemPath, $serializedItem);
+        $result = file_put_contents($itemPath, $serializedItem, LOCK_EX);
 
         // 0 bytes write is considered a successful operation
         if ($result === false) {

tests/Cache/FileSystemCacheItemPoolTest.php

@@ -21,10 +21,12 @@
 use Google\Auth\Cache\TypedItem;
 use PHPUnit\Framework\TestCase;
 use Psr\Cache\InvalidArgumentException;
+use Symfony\Component\Filesystem\Filesystem;
 
 class FileSystemCacheItemPoolTest extends TestCase
 {
-    private string $defaultCacheDirectory = '.cache';
+    private string $cachePath;
+    private Filesystem $filesystem;
     private FileSystemCacheItemPool $pool;
     private array $invalidChars = [
         '`', '~', '!', '@', '#', '$',
@@ -36,28 +38,21 @@ class FileSystemCacheItemPoolTest extends TestCase
 
     public function setUp(): void
     {
-        $this->pool = new FileSystemCacheItemPool($this->defaultCacheDirectory);
+        $this->cachePath = sys_get_temp_dir() . '/google_auth_php_test/';
+        $this->filesystem = new Filesystem();
+        $this->filesystem->remove($this->cachePath);
+        $this->pool = new FileSystemCacheItemPool($this->cachePath);
     }
 
     public function tearDown(): void
     {
-        $files = scandir($this->defaultCacheDirectory);
-
-        foreach ($files as $fileName) {
-            if ($fileName === '.' || $fileName === '..') {
-                continue;
-            }
-
-            unlink($this->defaultCacheDirectory . '/' . $fileName);
-        }
-
-        rmdir($this->defaultCacheDirectory);
+        $this->filesystem->remove($this->cachePath);
     }
 
     public function testInstanceCreatesCacheFolder()
     {
-        $this->assertTrue(file_exists($this->defaultCacheDirectory));
-        $this->assertTrue(is_dir($this->defaultCacheDirectory));
+        $this->assertTrue(file_exists($this->cachePath));
+        $this->assertTrue(is_dir($this->cachePath));
     }
 
     public function testSaveAndGetItem()
@@ -134,10 +129,10 @@ public function testClear()
     {
         $item = $this->getNewItem();
         $this->pool->save($item);
-        $this->assertLessThan(scandir($this->defaultCacheDirectory), 2);
+        $this->assertLessThan(scandir($this->cachePath), 2);
         $this->pool->clear();
         // Clear removes all the files, but scandir returns `.` and `..` as files
-        $this->assertEquals(count(scandir($this->defaultCacheDirectory)), 2);
+        $this->assertEquals(count(scandir($this->cachePath)), 2);
     }
 
     public function testSaveDeferredAndCommit()

tests/Cache/RaceConditionTest.php

@@ -0,0 +1,113 @@
+<?php
+/*
+ * Copyright 2025 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+namespace Google\Auth\Tests\Cache;
+
+use Google\Auth\Cache\FileSystemCacheItemPool;
+use Google\Auth\Cache\MemoryCacheItemPool;
+use Google\Auth\Cache\SysVCacheItemPool;
+use PHPUnit\Framework\TestCase;
+use Psr\Cache\CacheItemPoolInterface;
+use Symfony\Component\Filesystem\Filesystem;
+
+class RaceConditionTest extends TestCase
+{
+    private static string $cachePath;
+    private static Filesystem $filesystem;
+
+    public static function setUpBeforeClass(): void
+    {
+        self::$cachePath = sys_get_temp_dir() . '/google_auth_php_test/';
+        self::$filesystem = new Filesystem();
+        self::$filesystem->remove(self::$cachePath);
+    }
+
+    /**
+     * @runInSeparateProcess
+     * @dataProvider provideRaceCondition
+     */
+    public function testRaceCondition(string $cacheClass)
+    {
+        if (!function_exists('pcntl_fork')) {
+            $this->markTestSkipped('pcntl_fork is not available');
+        }
+        for ($i = 0; $i < 100; $i++) {
+
+            $pids = [];
+            for ($j = 0; $j < 4; $j++) {
+                $pid = pcntl_fork();
+                if ($pid == -1) {
+                    $this->fail('Could not fork');
+                }
+                $pool = $this->createCacheItemPool($cacheClass);
+                $item = $pool->getItem('foo');
+                $item->set('bar');
+                $pool->save($item);
+
+                if ($pid) {
+                    // parent
+                    $pids[] = $pid;
+                } else {
+                    // child
+                    exit(0);
+                }
+            }
+
+            // parent
+            $pool->save($item);
+
+            foreach ($pids as $pid) {
+                pcntl_waitpid($pid, $status);
+                $this->assertEquals(0, $status);
+            }
+
+            $this->assertTrue($pool->hasItem('foo'));
+            $cachedItem = $pool->getItem('foo');
+            $this->assertEquals('bar', $cachedItem->get());
+        }
+    }
+
+    public function createCacheItemPool(string $cacheClass): CacheItemPoolInterface
+    {
+        switch ($cacheClass) {
+            case FileSystemCacheItemPool::class:
+                $cachePath = self::$cachePath . '/google_auth_php_test-' . rand();
+                return new FileSystemCacheItemPool($cachePath);
+            case MemoryCacheItemPool::class:
+                return new MemoryCacheItemPool();
+            case SysVCacheItemPool::class:
+                return new SysVCacheItemPool();
+        }
+
+        throw new \Exception('Unrecognized cache class: ' . $cacheClass);
+    }
+
+    public function provideRaceCondition()
+    {
+        return [
+            [FileSystemCacheItemPool::class],
+            [MemoryCacheItemPool::class],
+            [SysVCacheItemPool::class],
+        ];
+    }
+
+    public static function tearDownAfterClass(): void
+    {
+        // remove all files generated from the filecaches
+        self::$filesystem->remove(self::$cachePath);
+    }
+}