diff --git a/src/View/CsvView.php b/src/View/CsvView.php index 679587c..60ec9c7 100644 --- a/src/View/CsvView.php +++ b/src/View/CsvView.php @@ -7,6 +7,7 @@ use Cake\Datasource\EntityInterface; use Cake\Utility\Hash; use Cake\View\SerializedView; +use Stringable; /** * A view class that is used for CSV responses. @@ -77,11 +78,18 @@ class CsvView extends SerializedView protected string $subDir = 'csv'; /** - * Whether or not to reset static variables in use + * Aggregated CSV output for the current serialization pass. * - * @var bool + * @var string + */ + protected string $csv = ''; + + /** + * Temp stream used by fputcsv() to generate a single row. + * + * @var resource|null */ - protected bool $_resetStaticVariables = false; + protected $fp = null; /** * Iconv extension. @@ -201,16 +209,43 @@ public static function contentType(): string */ protected function _serialize(array|string $serialize): string { + $this->resetState(); + $this->_renderRow($this->getConfig('header')); $this->_renderContent(); $this->_renderRow($this->getConfig('footer')); - $content = $this->_renderRow(); - $this->_resetStaticVariables = true; - $this->_renderRow(); + $content = $this->csv; + + $this->resetState(); return $content; } + /** + * Reset accumulated state so the same view instance can render multiple + * times in a single request (queue worker, multi-file export, etc.). + */ + protected function resetState(): void + { + $this->csv = ''; + $this->isFirstBom = true; + if (is_resource($this->fp)) { + fclose($this->fp); + } + $this->fp = null; + } + + /** + * @inheritDoc + */ + public function __destruct() + { + if (is_resource($this->fp)) { + fclose($this->fp); + $this->fp = null; + } + } + /** * Renders the body of the data to the csv * @@ -245,24 +280,35 @@ protected function _renderContent(): void foreach ($extract as $formatter) { if (!is_string($formatter) && is_callable($formatter)) { $value = $formatter($_data); + $pathForError = ''; } else { $path = $formatter; $format = null; if (is_array($formatter)) { [$path, $format] = $formatter; } + $pathForError = (string)$path; - if (!str_contains($path, '.')) { - $value = $_data[$path]; - } else { - $value = Hash::get($_data, $path); - } + $value = Hash::get($_data, $path); - if ($format) { + if ($format !== null) { $value = sprintf($format, $value); } } + if ( + $value !== null + && !is_scalar($value) + && !($value instanceof Stringable) + ) { + throw new CakeException(sprintf( + 'Extract path `%s` resolved to a non-scalar `%s`. ' + . 'Use a callable formatter to flatten it, or adjust the extract path.', + $pathForError, + get_debug_type($value), + )); + } + $values[] = $value; } $this->_renderRow($values); @@ -273,23 +319,14 @@ protected function _renderContent(): void /** * Aggregates the rows into a single csv * - * @param array|null $row Row data + * @param array|null $row Row data * @return string CSV with all data to date */ protected function _renderRow(?array $row = null): string { - static $csv = ''; + $this->csv .= (string)$this->_generateRow($row); - if ($this->_resetStaticVariables) { - $csv = ''; - $this->_resetStaticVariables = false; - - return ''; - } - - $csv .= (string)$this->_generateRow($row); - - return $csv; + return $this->csv; } /** @@ -297,30 +334,29 @@ protected function _renderRow(?array $row = null): string * data by writing the array to a temporary file and * returning its contents * - * @param array|null $row Row data + * @param array|null $row Row data * @return string|false String with the row in csv-syntax, false on fputscv failure */ protected function _generateRow(?array $row = null): string|false { - static $fp = false; - if (!$row) { return ''; } - if ($fp === false) { + if ($this->fp === null) { $stream = 'php://temp'; $fp = fopen($stream, 'r+'); if ($fp === false) { throw new CakeException(sprintf('Cannot open stream `%s`', $stream)); } + $this->fp = $fp; $setSeparator = $this->getConfig('setSeparator'); if ($setSeparator) { - fwrite($fp, 'sep=' . $setSeparator . "\n"); + fwrite($this->fp, 'sep=' . $setSeparator . "\n"); } } else { - ftruncate($fp, 0); + ftruncate($this->fp, 0); } $null = $this->getConfig('null'); @@ -340,21 +376,21 @@ protected function _generateRow(?array $row = null): string|false /** @phpstan-ignore-next-line */ $row = str_replace(["\r\n", "\n", "\r"], $newline, $row); if ($enclosure === '') { - // fputcsv does not supports empty enclosure - if (fputs($fp, implode($delimiter, $row) . "\n") === false) { + // fputcsv does not support empty enclosure + if (fputs($this->fp, implode($delimiter, $row) . "\n") === false) { return false; } } else { - if (fputcsv($fp, $row, $delimiter, $enclosure, $escape) === false) { + if (fputcsv($this->fp, $row, $delimiter, $enclosure, $escape) === false) { return false; } } - rewind($fp); + rewind($this->fp); unset($row); $csv = ''; - while (($buffer = fgets($fp, 4096)) !== false) { + while (($buffer = fgets($this->fp, 4096)) !== false) { $csv .= $buffer; } diff --git a/tests/TestCase/View/CsvViewTest.php b/tests/TestCase/View/CsvViewTest.php index 2c2b11c..117eb32 100644 --- a/tests/TestCase/View/CsvViewTest.php +++ b/tests/TestCase/View/CsvViewTest.php @@ -3,6 +3,7 @@ namespace CsvView\Test\TestCase\View; +use Cake\Core\Exception\CakeException; use Cake\Http\Response; use Cake\Http\ServerRequest as Request; use Cake\I18n\DateTime; @@ -519,4 +520,81 @@ public function testInvalidViewVarThrowsException() $this->view->setConfig('serialize', 'data'); $this->view->render(); } + + /** + * Rendering the same instance twice must produce clean output both times + * (no stale BOM state, no leftover writer state). + * + * @return void + */ + public function testRenderTwiceWithSameInstance() + { + $data = [['a', 'b'], ['c', 'd']]; + $this->view->set(['data' => $data]) + ->setConfig(['serialize' => 'data', 'bom' => true, 'csvEncoding' => 'UTF-8']); + + $bom = chr(0xEF) . chr(0xBB) . chr(0xBF); + $expected = $bom . 'a,b' . PHP_EOL . 'c,d' . PHP_EOL; + + $this->assertSame($expected, $this->view->render()); + $this->assertSame($expected, $this->view->render()); + } + + /** + * Simple (non-dotted) extract paths must fall through Hash::get() so a + * missing key resolves to null instead of triggering an undefined-key + * warning. + * + * @return void + */ + public function testRenderViaExtractMissingSimpleKey() + { + $data = [ + ['name' => 'alice', 'email' => 'a@example.com'], + ['name' => 'bob'], // missing 'email' + ]; + $this->view->set(['users' => $data]) + ->setConfig([ + 'serialize' => 'users', + 'extract' => ['name', 'email'], + ]); + + $expected = 'alice,a@example.com' . PHP_EOL . 'bob,' . PHP_EOL; + $this->assertSame($expected, $this->view->render()); + } + + /** + * An extract path that resolves to an array (e.g. a hasMany association) + * must throw a clear exception instead of silently producing "Array to + * string conversion" notices and corrupted CSV. Regression for #131. + * + * @return void + */ + public function testRenderViaExtractArrayValueThrows() + { + $data = [ + [ + 'id' => 1, + 'tags' => [['name' => 'php'], ['name' => 'cakephp']], + ], + ]; + $this->view->set(['rows' => $data]) + ->setConfig([ + 'serialize' => 'rows', + 'extract' => ['id', 'tags'], + ]); + + try { + $this->view->render(); + $this->fail('Expected exception for array-valued extract path.'); + } catch (Exception $e) { + // SerializedView wraps our CakeException in SerializationFailureException. + $previous = $e->getPrevious() ?? $e; + $this->assertInstanceOf(CakeException::class, $previous); + $this->assertStringContainsString( + 'Extract path `tags` resolved to a non-scalar `array`', + $previous->getMessage(), + ); + } + } }