Skip to content

Commit 925b07a

Browse files
nyamsprodnyamsprod
authored
Fix issue #586 (#587) Rewrite BOM handling in the Reader class
* Fix BOM handling #586 Rewrite BOM handling in the Reader class Introduce the SkipBomIterator to centralize BOM handling inside the Reader class.
1 parent 1e7bb18 commit 925b07a

4 files changed

Lines changed: 223 additions & 92 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ All Notable changes to `Csv` will be documented in this file
1818

1919
- `Writer::forceEnclosure` now doubles embedded enclosures when the escape character is the empty string.
2020
- `EscapeFormula` round-trip now preserves fields starting with the escape character
21+
- BOM handling is re-written to fix [#586]([https://github](https://github.com/thephpleague/csv/issues/586))
2122

2223
### Remove
2324

‎src/Reader.php‎

Lines changed: 117 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,14 @@
2222
use League\Csv\Serializer\MappingFailed;
2323
use League\Csv\Serializer\TypeCastingFailed;
2424
use ReflectionException;
25+
use SeekableIterator;
2526
use SplFileObject;
2627

2728
use function array_filter;
2829
use function array_reduce;
2930
use function array_unique;
3031
use function is_array;
32+
use function is_string;
3133
use function iterator_count;
3234
use function strlen;
3335
use function substr;
@@ -152,77 +154,60 @@ public function getHeader(): array
152154
*/
153155
protected function setHeader(int $offset): array
154156
{
155-
$inputBom = null;
156-
$header = $this->seekRow($offset);
157-
if (0 === $offset) {
158-
$inputBom = Bom::tryFrom($this->getInputBOM());
159-
$header = $this->removeBOM(
160-
$header,
161-
!$this->is_input_bom_included ? $inputBom?->length() ?? 0 : 0,
162-
$this->enclosure
163-
);
157+
$header = [];
158+
foreach ($this->getInnerIterator() as $key => $record) {
159+
if ($key === $offset) {
160+
$header = $record;
161+
break;
162+
}
164163
}
165164

166165
return match (true) {
167166
[] === $header,
168167
[null] === $header,
169168
[false] === $header,
170-
[''] === $header && 0 === $offset && null !== $inputBom => throw SyntaxError::dueToHeaderNotFound($offset),
169+
[''] === $header && 0 === $offset => throw SyntaxError::dueToHeaderNotFound($offset),
171170
default => $header,
172171
};
173172
}
174173

175174
/**
176175
* @throws Exception
177176
*
178-
* Returns the row at a given offset.
177+
* @return CallbackFilterIterator<int, array, SeekableIterator>
179178
*/
180-
protected function seekRow(int $offset): array
179+
protected function getInnerIterator(): CallbackFilterIterator
181180
{
182-
$this->getDocument()->seek($offset);
183-
$record = $this->document->current();
184-
185-
return match (true) {
186-
false === $record => [],
187-
default => (array) $record,
188-
};
181+
return new CallbackFilterIterator(
182+
$this->getSeekableIterator(),
183+
fn ($record): bool => is_array($record) && ($this->is_empty_records_included || $record !== [null])
184+
);
189185
}
190186

191187
/**
192188
* @throws Exception
193189
*
194-
* Returns the document as an Iterator.
190+
* @return SeekableIterator<int, array>
195191
*/
196-
protected function getDocument(): SplFileObject|Stream
192+
protected function getSeekableIterator(): SeekableIterator
197193
{
198-
$this->document->setFlags(SplFileObject::READ_CSV | SplFileObject::READ_AHEAD);
199-
$this->document->setCsvControl($this->delimiter, $this->enclosure, $this->escape);
200-
$this->document->rewind();
194+
$document = $this->getDocument();
201195

202-
return $this->document;
196+
return $this->is_input_bom_included ? $document : new SkipBomIterator($document);
203197
}
204198

205199
/**
206-
* Strips the BOM sequence from a record.
207-
*
208-
* @param array<string> $record
200+
* @throws Exception
209201
*
210-
* @return array<string>
202+
* Returns the document as an Iterator.
211203
*/
212-
protected function removeBOM(array $record, int $bom_length, string $enclosure): array
204+
protected function getDocument(): SplFileObject|Stream
213205
{
214-
if ([] === $record || !is_string($record[0]) || 0 === $bom_length || strlen($record[0]) < $bom_length) {
215-
return $record;
216-
}
217-
218-
$record[0] = substr($record[0], $bom_length);
219-
if ($enclosure.$enclosure !== substr($record[0].$record[0], strlen($record[0]) - 1, 2)) {
220-
return $record;
221-
}
222-
223-
$record[0] = substr($record[0], 1, -1);
206+
$this->document->setFlags(SplFileObject::READ_CSV | SplFileObject::READ_AHEAD);
207+
$this->document->setCsvControl($this->delimiter, $this->enclosure, $this->escape);
208+
$this->document->rewind();
224209

225-
return $record;
210+
return $this->document;
226211
}
227212

228213
public function fetchColumn(string|int $index = 0): Iterator
@@ -245,18 +230,17 @@ public function first(): array
245230

246231
protected function getLastRecord(array $header): array
247232
{
248-
$this->document->setFlags(SplFileObject::READ_CSV);
249-
$this->document->setCsvControl($this->delimiter, $this->enclosure, $this->escape);
250-
$this->document->seek(PHP_INT_MAX);
251-
$offset = $this->document->key();
233+
$document = $this->getSeekableIterator();
234+
$document->seek(PHP_INT_MAX);
235+
$offset = $document->key();
252236
$row = false;
253237
for (; $offset >= 0; --$offset) {
254238
if ($this->header_offset === $offset) {
255239
continue;
256240
}
257-
$this->document->seek($offset);
241+
$document->seek($offset);
258242
/** @var array|false $row */
259-
$row = $this->document->current();
243+
$row = $document->current();
260244
if ($row !== [null] && false !== $row) {
261245
break;
262246
}
@@ -266,10 +250,6 @@ protected function getLastRecord(array $header): array
266250
return [];
267251
}
268252

269-
if (0 === $offset) {
270-
$row = $this->removeBOM($row, $this->input_bom?->length() ?? 0, $this->enclosure);
271-
}
272-
273253
$formatter = fn (array $record): array => array_reduce(
274254
$this->formatters,
275255
fn (array $record, Closure $formatter): array => $formatter($record),
@@ -279,7 +259,6 @@ protected function getLastRecord(array $header): array
279259
$record = $row;
280260
if ([] === $header) {
281261
$header = $this->getHeader();
282-
;
283262
}
284263

285264
if ([] !== $header) {
@@ -533,10 +512,7 @@ public function selectAllExcept(string|int ...$columns): TabularDataReader
533512
*/
534513
public function getRecords(array $header = []): Iterator
535514
{
536-
return $this->combineHeader(
537-
$this->prepareRecords(),
538-
$this->prepareHeader($header)
539-
);
515+
return $this->combineHeader($this->prepareRecords(), $this->prepareHeader($header));
540516
}
541517

542518
/**
@@ -567,13 +543,7 @@ public function getRecordsAsObject(string $className, array $header = []): Itera
567543
*/
568544
protected function prepareRecords(): Iterator
569545
{
570-
$normalized = fn ($record): bool => is_array($record) && ($this->is_empty_records_included || $record !== [null]);
571-
$bom = null;
572-
if (!$this->is_input_bom_included) {
573-
$bom = Bom::tryFrom($this->getInputBOM());
574-
}
575-
576-
$records = $this->stripBOM(new CallbackFilterIterator($this->getDocument(), $normalized), $bom);
546+
$records = $this->getInnerIterator();
577547
if (null !== $this->header_offset) {
578548
$records = new CallbackFilterIterator($records, fn (array $record, int $offset): bool => $offset !== $this->header_offset);
579549
}
@@ -585,35 +555,6 @@ protected function prepareRecords(): Iterator
585555
return $records;
586556
}
587557

588-
/**
589-
* Strips the BOM sequence from the returned records if necessary.
590-
*/
591-
protected function stripBOM(Iterator $iterator, ?Bom $bom): Iterator
592-
{
593-
if (null === $bom) {
594-
return $iterator;
595-
}
596-
597-
$bomLength = $bom->length();
598-
$mapper = function (array $record, int $index) use ($bomLength): array {
599-
if (0 !== $index) {
600-
return $record;
601-
}
602-
603-
$record = $this->removeBOM($record, $bomLength, $this->enclosure);
604-
605-
return match ($record) {
606-
[''] => [null],
607-
default => $record,
608-
};
609-
};
610-
611-
return new CallbackFilterIterator(
612-
new MapIterator($iterator, $mapper),
613-
fn (array $record): bool => $this->is_empty_records_included || $record !== [null]
614-
);
615-
}
616-
617558
/**
618559
* @param array<string> $header
619560
*
@@ -671,6 +612,90 @@ protected function combineHeader(Iterator $iterator, array $header): Iterator
671612
};
672613
}
673614

615+
/**
616+
* DEPRECATION WARNING! This method will be removed in the next major point release.
617+
*
618+
* Returns the row at a given offset.
619+
*
620+
* @deprecated
621+
* @throws Exception
622+
*
623+
* @deprecated since version 9.29 without any replacement
624+
* @codeCoverageIgnore
625+
*/
626+
protected function seekRow(int $offset): array
627+
{
628+
$this->getDocument()->seek($offset);
629+
$record = $this->document->current();
630+
631+
return match (true) {
632+
false === $record => [],
633+
default => (array) $record,
634+
};
635+
}
636+
637+
/**
638+
* Strips the BOM sequence from a record.
639+
*
640+
* DEPRECATION WARNING! This method will be removed in the next major point release.
641+
*
642+
* @param array<string> $record
643+
*
644+
* @return array<string>
645+
*
646+
* @deprecated since version 9.29 without any replacement
647+
* @codeCoverageIgnore
648+
*/
649+
protected function removeBOM(array $record, int $bom_length, string $enclosure): array
650+
{
651+
if ([] === $record || !is_string($record[0]) || 0 === $bom_length || strlen($record[0]) < $bom_length) {
652+
return $record;
653+
}
654+
655+
$record[0] = substr($record[0], $bom_length);
656+
if ($enclosure.$enclosure !== substr($record[0].$record[0], strlen($record[0]) - 1, 2)) {
657+
return $record;
658+
}
659+
660+
$record[0] = substr($record[0], 1, -1);
661+
662+
return $record;
663+
}
664+
665+
/**
666+
* Strips the BOM sequence from the returned records if necessary.
667+
*
668+
* DEPRECATION WARNING! This method will be removed in the next major point release.
669+
*
670+
* @deprecated since version 9.29 without any replacement
671+
* @codeCoverageIgnore
672+
*/
673+
protected function stripBOM(Iterator $iterator, ?Bom $bom): Iterator
674+
{
675+
if (null === $bom) {
676+
return $iterator;
677+
}
678+
679+
$bomLength = $bom->length();
680+
$mapper = function (array $record, int $index) use ($bomLength): array {
681+
if (0 !== $index) {
682+
return $record;
683+
}
684+
685+
$record = $this->removeBOM($record, $bomLength, $this->enclosure);
686+
687+
return match ($record) {
688+
[''] => [null],
689+
default => $record,
690+
};
691+
};
692+
693+
return new CallbackFilterIterator(
694+
new MapIterator($iterator, $mapper),
695+
fn (array $record): bool => $this->is_empty_records_included || $record !== [null]
696+
);
697+
}
698+
674699
/**
675700
* DEPRECATION WARNING! This method will be removed in the next major point release.
676701
*

‎src/ReaderTest.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -652,4 +652,26 @@ public function testStreamWithFiltersDestructsGracefully(): void
652652
// An explicitly closed file handle makes the stream filter resources invalid
653653
fclose($fp);
654654
}
655+
656+
public function test_handline_bom_expression_in_records_issue_586(): void
657+
{
658+
$bom = Bom::Utf8->value;
659+
$contents = [
660+
'"a,b",c' => ["a,b", "c"],
661+
'"a'."\n".'b",c' => ['a'."\n".'b', "c"],
662+
];
663+
664+
foreach ($contents as $content => $expected) {
665+
self::assertSame($expected, Reader::fromString($content)->first());
666+
self::assertSame($expected, Reader::fromString($bom.$content)->first());
667+
}
668+
}
669+
670+
public function test_handline_bom_expression_in_heasders_issue_586(): void
671+
{
672+
$csv = Reader::fromString(Bom::Utf8->value.'"a,b",c'."\n".'1,2');
673+
$csv->setHeaderOffset(0);
674+
675+
self::assertSame(["a,b","c"], $csv->getHeader());
676+
}
655677
}

0 commit comments

Comments
 (0)