Explorar el Código

Fix actualize mutex collision across instances (#9045)

## What changed

- derive the actualization mutex name from a SHA-256 hash of the canonical `DATA_PATH`
- keep `TMP_PATH` as the configurable location for the mutex file
- add coverage for deterministic per-instance names and independent locks

## Why

Multiple FreshRSS instances that use the default shared system temporary directory currently use the same actualization lock. The data path identifies the instance without exposing or depending on its configured salt.

## Validation

- `vendor/bin/phpunit --bootstrap ./tests/bootstrap.php ./tests/lib/ActualizeMutexTest.php --no-progress`
- `vendor/bin/phpcs app/actualize_script.php lib/lib_rss.php tests/lib/ActualizeMutexTest.php -s`
- `vendor/bin/phpstan analyse --memory-limit 512M --no-progress app/actualize_script.php lib/lib_rss.php tests/lib/ActualizeMutexTest.php`

Fixes #6370

Co-authored-by: Gerard Alvear <gerard.alvear@logiqd.me>
Gerard Alvear Porras hace 2 días
padre
commit
c15171efb5
Se han modificado 3 ficheros con 46 adiciones y 1 borrados
  1. 1 1
      app/actualize_script.php
  2. 10 0
      lib/lib_rss.php
  3. 35 0
      tests/lib/ActualizeMutexTest.php

+ 1 - 1
app/actualize_script.php

@@ -38,7 +38,7 @@ function notice(string $message): void {
 
 // <Mutex>
 // Avoid having multiple actualization processes at the same time
-$mutexFile = TMP_PATH . '/actualize.freshrss.lock';
+$mutexFile = actualize_mutex_file(TMP_PATH, DATA_PATH);
 $mutexTtl = 900; // seconds (refreshed before each new feed)
 if (file_exists($mutexFile) && ((time() - (@filemtime($mutexFile) ?: 0)) > $mutexTtl)) {
 	unlink($mutexFile);

+ 10 - 0
lib/lib_rss.php

@@ -37,6 +37,16 @@ function join_path(...$path_parts): string {
 	return join(DIRECTORY_SEPARATOR, $path_parts);
 }
 
+/**
+ * Build the mutex path for an actualisation run.
+ *
+ * The data path identifies a FreshRSS instance, while the temporary path only
+ * determines where its mutex is stored.
+ */
+function actualize_mutex_file(string $tmpPath, string $dataPath): string {
+	return $tmpPath . '/actualize.' . hash('sha256', realpath($dataPath) ?: $dataPath) . '.freshrss.lock';
+}
+
 //<Auto-loading>
 function classAutoloader(string $class): void {
 	if (str_starts_with($class, 'FreshRSS')) {

+ 35 - 0
tests/lib/ActualizeMutexTest.php

@@ -0,0 +1,35 @@
+<?php
+declare(strict_types=1);
+
+final class ActualizeMutexTest extends \PHPUnit\Framework\TestCase {
+	public function testMutexFilesAreUniquePerDataPathAndUseTheConfiguredTemporaryPath(): void {
+		$testPath = sys_get_temp_dir() . '/freshrss-actualize-mutex-' . bin2hex(random_bytes(8));
+		$tmpPath = $testPath . '/tmp';
+		$firstDataPath = $testPath . '/first/data';
+		$secondDataPath = $testPath . '/second/data';
+		mkdir($tmpPath, 0700, true);
+		mkdir($firstDataPath, 0700, true);
+		mkdir($secondDataPath, 0700, true);
+
+		try {
+			$firstMutex = actualize_mutex_file($tmpPath, $firstDataPath);
+			$secondMutex = actualize_mutex_file($tmpPath, $secondDataPath);
+
+			self::assertSame($firstMutex, actualize_mutex_file($tmpPath, $firstDataPath . '/.'));
+			self::assertNotSame($firstMutex, $secondMutex);
+			self::assertStringStartsWith($tmpPath . '/actualize.', $firstMutex);
+			self::assertStringEndsWith('.freshrss.lock', $firstMutex);
+
+			$firstHandle = fopen($firstMutex, 'x');
+			$secondHandle = fopen($secondMutex, 'x');
+			self::assertIsResource($firstHandle);
+			self::assertIsResource($secondHandle);
+			fclose($firstHandle);
+			fclose($secondHandle);
+
+			self::assertFalse(@fopen($firstMutex, 'x'));
+		} finally {
+			recursive_unlink($testPath);
+		}
+	}
+}