diff --git a/CHANGELOG.md b/CHANGELOG.md index 44d3f0cb..1eb52537 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## [Unreleased] + +### Fixed + +- Use a unique temporary file when exporting import errors to CSV (backport of PR #675 from GLPI 11-compatible line) + ## [2.14.5] 2026-09-11 ### Fixed diff --git a/inc/clientinjection.class.php b/inc/clientinjection.class.php index c9e3524c..bd1263b0 100644 --- a/inc/clientinjection.class.php +++ b/inc/clientinjection.class.php @@ -425,34 +425,87 @@ private static function escapeCsvFormula($value) return $value; } - public static function exportErrorsInCSV() + private static function writeErrorsCsv(string $dir, array $error_lines, array $headers, string $delimiter): string { + $upload_dir = realpath($dir); + // tempnam() silently falls back to the system temp dir when the target dir is unusable + if ($upload_dir === false || !is_writable($upload_dir)) { + throw new RuntimeException(sprintf('Upload directory "%s" is not writable.', $dir)); + } - $error_lines = json_decode(PluginDatainjectionSession::getParam('error_lines'), true); - self::stripslashes_array($error_lines); + $file = tempnam($upload_dir, 'ERR'); + if ($file === false) { + throw new RuntimeException(sprintf('Unable to create a temporary file in "%s".', $dir)); + } - if (!empty($error_lines)) { - $model = unserialize(PluginDatainjectionSession::getParam('currentmodel')); - $file = PLUGIN_DATAINJECTION_UPLOAD_DIR . basename(PluginDatainjectionSession::getParam('file_name')); + $tmpfile = fopen($file, 'w'); + if ($tmpfile === false) { + self::discardErrorsCsv(null, $file); + throw new RuntimeException(sprintf('Unable to open "%s" for writing.', $file)); + } - $mappings = $model->getMappings(); - $tmpfile = fopen($file, 'w'); + $rows = $headers !== [] ? [$headers] : []; + foreach ($error_lines as $line) { + $rows[] = array_map([self::class, 'escapeCsvFormula'], $line); + } - //If headers present - if ($model->getBackend()->isHeaderPresent()) { - $headers = PluginDatainjectionMapping::getMappingsSortedByRank($model->fields['id']); - fputcsv($tmpfile, $headers, $model->getBackend()->getDelimiter()); + foreach ($rows as $row) { + if (fputcsv($tmpfile, $row, $delimiter, '"', '\\') === false) { + self::discardErrorsCsv($tmpfile, $file); + throw new RuntimeException(sprintf('Unable to write to "%s".', $file)); } + } + + if (!fclose($tmpfile)) { + self::discardErrorsCsv(null, $file); + throw new RuntimeException(sprintf('Unable to close "%s".', $file)); + } + + return $file; + } + + /** + * @param resource|null $handle + */ + private static function discardErrorsCsv($handle, string $file): void + { + // Cleanup failures are ignored so they never mask the original write error + if (is_resource($handle)) { + @fclose($handle); + } + + if (file_exists($file)) { + @unlink($file); + } + } + + public static function exportErrorsInCSV() + { - //Write lines - foreach ($error_lines as $line) { - fputcsv($tmpfile, array_map([self::class, 'escapeCsvFormula'], $line), $model->getBackend()->getDelimiter()); + $error_lines = json_decode(PluginDatainjectionSession::getParam('error_lines'), true); + self::stripslashes_array($error_lines); + + if (!empty($error_lines)) { + $model = unserialize(PluginDatainjectionSession::getParam('currentmodel')); + $backend = $model->getBackend(); + $headers = $backend->isHeaderPresent() + ? PluginDatainjectionMapping::getMappingsSortedByRank($model->fields['id']) + : []; + + try { + $file = self::writeErrorsCsv(PLUGIN_DATAINJECTION_UPLOAD_DIR, $error_lines, $headers, $backend->getDelimiter()); + } catch (RuntimeException $e) { + Toolbox::logError($e->getMessage()); + Session::addMessageAfterRedirect( + __('Unable to generate the error file', 'datainjection'), + false, + ERROR + ); + Html::back(); + return; } - fclose($tmpfile); - $name = "Error-" . basename(PluginDatainjectionSession::getParam('file_name')); - $name = str_replace(' ', '', $name); - header('Content-disposition: attachment; filename=' . $name); + header('Content-disposition: attachment; filename=Errors.csv'); header('Content-Type: application/octet-stream'); header('Content-Transfer-Encoding: fichier'); header('Content-Length: ' . filesize($file)); diff --git a/tests/unit/ClientInjectionWriteErrorsCsvTest.php b/tests/unit/ClientInjectionWriteErrorsCsvTest.php new file mode 100644 index 00000000..014ce6f4 --- /dev/null +++ b/tests/unit/ClientInjectionWriteErrorsCsvTest.php @@ -0,0 +1,124 @@ +. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2007-2023 by DataInjection plugin team. + * @license GPLv2 https://www.gnu.org/licenses/gpl-2.0.html + * @link https://github.com/pluginsGLPI/datainjection + * ------------------------------------------------------------------------- + */ + +require_once dirname(__DIR__, 2) . '/inc/clientinjection.class.php'; + +class ClientInjectionWriteErrorsCsvTest extends DbTestCase +{ + /** @var string[] */ + private array $created_files = []; + + public function tearDown(): void + { + foreach ($this->created_files as $created_file) { + if (file_exists($created_file)) { + unlink($created_file); + } + } + + $this->created_files = []; + + parent::tearDown(); + } + + private function writeErrorsCsv(array $error_lines, array $headers, string $dir = PLUGIN_DATAINJECTION_UPLOAD_DIR): string + { + $file = $this->invokePrivateStatic('writeErrorsCsv', [$dir, $error_lines, $headers, ';']); + $this->created_files[] = $file; + + return $file; + } + + public function testFileIsCreatedInUploadDirWithErrPrefix(): void + { + $file = $this->writeErrorsCsv([['a', 'b']], []); + + $this->assertSame(realpath(PLUGIN_DATAINJECTION_UPLOAD_DIR), realpath(dirname($file))); + $this->assertStringStartsWith('ERR', basename($file)); + } + + public function testMissingDirDoesNotFallBackToSystemTempDir(): void + { + $this->expectException(RuntimeException::class); + + $this->writeErrorsCsv([['a']], [], PLUGIN_DATAINJECTION_UPLOAD_DIR . '/missing_dir'); + } + + public function testEachCallUsesADistinctFile(): void + { + $first = $this->writeErrorsCsv([['a']], []); + $second = $this->writeErrorsCsv([['a']], []); + + $this->assertNotSame($first, $second); + } + + public function testContentContainsHeadersAndEscapedLines(): void + { + $file = $this->writeErrorsCsv([['=SUM(A1)', 'safe']], ['name', 'serial']); + + $this->assertSame("name;serial\n'=SUM(A1);safe\n", file_get_contents($file)); + } + + public function testDiscardClosesHandleAndRemovesFile(): void + { + $file = $this->writeErrorsCsv([['a']], []); + $handle = fopen($file, 'r'); + + $this->discardErrorsCsv($handle, $file); + + $this->assertFalse(is_resource($handle)); + $this->assertFileDoesNotExist($file); + } + + public function testDiscardIgnoresCleanupFailures(): void + { + $missing_file = PLUGIN_DATAINJECTION_UPLOAD_DIR . '/missing_file.csv'; + + $this->discardErrorsCsv(null, $missing_file); + + $this->assertFileDoesNotExist($missing_file); + } + + private function discardErrorsCsv($handle, string $file): void + { + $this->invokePrivateStatic('discardErrorsCsv', [$handle, $file]); + } + + private function invokePrivateStatic(string $method, array $args) + { + $reflection = new ReflectionMethod(PluginDatainjectionClientInjection::class, $method); + // setAccessible() is required on PHP < 8.1 and deprecated on PHP >= 8.5 + if (PHP_VERSION_ID < 80100) { + $reflection->setAccessible(true); + } + + return $reflection->invokeArgs(null, $args); + } +}