Skip to content

Commit a7c68d5

Browse files
authored
Fix CsvReader::current() TypeError at EOF (#18)
Supersedes #3 with regression tests.
1 parent e3f743d commit a7c68d5

2 files changed

Lines changed: 97 additions & 3 deletions

File tree

‎src/CsvReader.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,11 @@ public function current(): ?array
107107
return $this->file->current();
108108
}
109109

110-
// Since the CSV has column headers use them to construct an associative array for the columns in this line
111-
do {
110+
// Since the CSV has column headers use them to construct an associative array for the columns in this line.
111+
// Check valid() before current(): SplFileObject::current() returns false at EOF, and a do-while would
112+
// still enter the body once when the iterator is already invalid (e.g. OneToManyReader calls current()
113+
// after next() past the last detail row), causing count(false) TypeError on PHP 8+.
114+
while ($this->valid()) {
112115
$line = $this->file->current();
113116

114117
// In non-strict mode pad/slice the line to match the column headers
@@ -135,7 +138,7 @@ public function current(): ?array
135138
$this->errors[$this->key()] = $line;
136139
$this->next();
137140
}
138-
} while($this->valid());
141+
}
139142

140143
return null;
141144
}

‎tests/CsvReaderTest.php‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,97 @@ public function testMaximumNesting()
265265
}
266266
}
267267

268+
/**
269+
* When the iterator is already at EOF, current() must return null rather than
270+
* calling count() on SplFileObject's false (do-while entered once while invalid).
271+
*
272+
* @see https://github.com/portphp/csv/pull/3
273+
*/
274+
public function testCurrentAtEndOfFileWithHeadersReturnsNull()
275+
{
276+
$file = new \SplTempFileObject();
277+
$file->fwrite("id,name\n1,Alice\n2,Bob\n");
278+
$file->rewind();
279+
280+
$reader = new CsvReader($file);
281+
$reader->setHeaderRowNumber(0);
282+
283+
// Exhaust the iterator
284+
iterator_to_array($reader);
285+
286+
$this->assertFalse($reader->valid());
287+
$this->assertNull($reader->current());
288+
}
289+
290+
/**
291+
* getRow() past the last line should not TypeError on count(false).
292+
*
293+
* @see https://github.com/portphp/csv/pull/3
294+
*/
295+
public function testGetRowPastEndWithHeadersReturnsNull()
296+
{
297+
$file = new \SplTempFileObject();
298+
$file->fwrite("id,name\n1,Alice\n");
299+
$file->rewind();
300+
301+
$reader = new CsvReader($file);
302+
$reader->setHeaderRowNumber(0);
303+
304+
$this->assertNull($reader->getRow(99));
305+
}
306+
307+
/**
308+
* OneToManyReader calls rightReader->current() after next() past the last
309+
* detail row. Without checking valid() first, CsvReader::current() hit
310+
* count(false) and broke joins on the last master row.
311+
*
312+
* This is the real-world failure reported against PR #3.
313+
*
314+
* @see https://github.com/portphp/csv/pull/3
315+
* @see https://github.com/portphp/csv/pull/3#issuecomment-769855101
316+
*/
317+
public function testOneToManyReaderConsumesLastDetailRowWithoutError()
318+
{
319+
$masterFile = new \SplTempFileObject();
320+
$masterFile->fwrite("id,name\n1,Alice\n2,Bob\n");
321+
$masterFile->rewind();
322+
$masterReader = new CsvReader($masterFile);
323+
$masterReader->setHeaderRowNumber(0);
324+
325+
$detailFile = new \SplTempFileObject();
326+
$detailFile->fwrite("id,item\n1,apple\n1,banana\n2,carrot\n");
327+
$detailFile->rewind();
328+
$detailReader = new CsvReader($detailFile);
329+
$detailReader->setHeaderRowNumber(0);
330+
331+
$reader = new \Port\Reader\OneToManyReader(
332+
$masterReader,
333+
$detailReader,
334+
'items',
335+
'id',
336+
'id'
337+
);
338+
339+
$rows = iterator_to_array($reader);
340+
341+
$this->assertCount(2, $rows);
342+
$this->assertEquals('Alice', $rows[1]['name']);
343+
$this->assertEquals(
344+
array(
345+
array('id' => '1', 'item' => 'apple'),
346+
array('id' => '1', 'item' => 'banana'),
347+
),
348+
$rows[1]['items']
349+
);
350+
$this->assertEquals('Bob', $rows[2]['name']);
351+
$this->assertEquals(
352+
array(
353+
array('id' => '2', 'item' => 'carrot'),
354+
),
355+
$rows[2]['items']
356+
);
357+
}
358+
268359
protected function getReader($filename)
269360
{
270361
$file = new \SplFileObject(__DIR__.'/fixtures/'.$filename);

0 commit comments

Comments
 (0)