diff --git a/packages/guides-graphs/src/Graphs/Renderer/PlantumlRenderer.php b/packages/guides-graphs/src/Graphs/Renderer/PlantumlRenderer.php index 73979549c..928ef9f36 100644 --- a/packages/guides-graphs/src/Graphs/Renderer/PlantumlRenderer.php +++ b/packages/guides-graphs/src/Graphs/Renderer/PlantumlRenderer.php @@ -25,6 +25,7 @@ use function mkdir; use function sys_get_temp_dir; use function tempnam; +use function unlink; final class PlantumlRenderer implements DiagramRenderer { @@ -53,11 +54,25 @@ public function render(RenderContext $renderContext, string $diagram): string|nu @enduml PUML; - if (!is_dir($this->tempDirectory)) { - mkdir($this->tempDirectory, 0o755, true); + if (!$this->ensureDirectoryExists($this->tempDirectory)) { + $this->logger->error( + 'Failed to create temp directory: ' . $this->tempDirectory, + $renderContext->getLoggerInformation(), + ); + + return null; } $pumlFileLocation = tempnam($this->tempDirectory, 'pu_'); + if ($pumlFileLocation === false) { + $this->logger->error( + 'Failed to create temporary file for diagram', + $renderContext->getLoggerInformation(), + ); + + return null; + } + file_put_contents($pumlFileLocation, $output); try { $process = new Process([$this->plantUmlBinaryPath, '-tsvg', $pumlFileLocation], __DIR__, null, null, 600.0); @@ -86,6 +101,31 @@ public function render(RenderContext $renderContext, string $diagram): string|nu return null; } - return file_get_contents($pumlFileLocation . '.svg') ?: null; + $svg = file_get_contents($pumlFileLocation . '.svg') ?: null; + + @unlink($pumlFileLocation); + @unlink($pumlFileLocation . '.svg'); + + return $svg; + } + + /** + * Ensures the directory exists, handling race conditions safely. + * + * @return bool True if directory exists or was created, false on failure + */ + private function ensureDirectoryExists(string $directory): bool + { + if (is_dir($directory)) { + return true; + } + + // Attempt to create the directory (suppress warning if concurrent process creates it) + if (@mkdir($directory, 0o755, true)) { + return true; + } + + // mkdir failed - check if another process created it (race condition) + return is_dir($directory); } } diff --git a/packages/guides-graphs/tests/unit/Renderer/PlantumlRendererTest.php b/packages/guides-graphs/tests/unit/Renderer/PlantumlRendererTest.php index e94913890..37427506a 100644 --- a/packages/guides-graphs/tests/unit/Renderer/PlantumlRendererTest.php +++ b/packages/guides-graphs/tests/unit/Renderer/PlantumlRendererTest.php @@ -13,35 +13,93 @@ namespace phpDocumentor\Guides\Graphs\Renderer; +use FilesystemIterator; use phpDocumentor\Guides\RenderContext; use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; +use RecursiveDirectoryIterator; +use RecursiveIteratorIterator; +use SplFileInfo; +use Throwable; +use function assert; +use function basename; +use function is_dir; +use function realpath; use function rmdir; use function sys_get_temp_dir; use function uniqid; +use function unlink; + +use const DIRECTORY_SEPARATOR; final class PlantumlRendererTest extends TestCase { + private const TEMP_DIR_PREFIX = 'plantuml-test-'; + public function testRenderCreatesTempDirectoryWhenMissing(): void { - $tempDir = sys_get_temp_dir() . '/plantuml-test-' . uniqid(); + $tempDir = sys_get_temp_dir() . '/' . self::TEMP_DIR_PREFIX . uniqid('', true); - // Ensure the directory does not exist self::assertDirectoryDoesNotExist($tempDir); - // Use a non-existent binary path - the render will fail but directory should be created first $renderer = new PlantumlRenderer(new NullLogger(), '/non/existent/plantuml', $tempDir); $renderContext = $this->createMock(RenderContext::class); $renderContext->method('getLoggerInformation')->willReturn([]); - // The render will fail due to missing binary, but the temp directory should be created - $renderer->render($renderContext, 'A -> B'); + try { + // Render will fail (returns null) or throw due to non-existent plantuml binary. + // We only care about verifying that the temp directory was created. + $renderer->render($renderContext, 'A -> B'); + } catch (Throwable) { + // Expected: plantuml binary doesn't exist + } + + self::assertDirectoryExists($tempDir, 'Temp directory should have been created'); + + $this->removeTempDirSafely($tempDir); + } + + private function removeTempDirSafely(string $dir): void + { + if ($dir === '' || !is_dir($dir)) { + return; + } + + $realDir = realpath($dir); + if ($realDir === false) { + return; + } + + $realSysTmp = realpath(sys_get_temp_dir()); + if ($realSysTmp === false) { + return; + } + + // Safety: must be under system temp and have our prefix + self::assertStringStartsWith( + $realSysTmp . DIRECTORY_SEPARATOR, + $realDir . DIRECTORY_SEPARATOR, + 'Refusing to delete directory outside system temp', + ); + self::assertStringContainsString( + self::TEMP_DIR_PREFIX, + basename($realDir), + 'Refusing to delete directory without expected prefix', + ); + + /** @var RecursiveIteratorIterator $iterator */ + $iterator = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($realDir, FilesystemIterator::SKIP_DOTS), + RecursiveIteratorIterator::CHILD_FIRST, + ); - self::assertDirectoryExists($tempDir); + foreach ($iterator as $file) { + assert($file instanceof SplFileInfo); + $file->isDir() ? @rmdir($file->getPathname()) : @unlink($file->getPathname()); + } - // Clean up - @rmdir($tempDir); + @rmdir($realDir); } }