8000 [CSV-224] Some Multi Iterator Parsing Peek Sequences Incorrectly Consume · piomar123/commons-csv@f368f64 · GitHub
Skip to content

Commit f368f64

Browse files
davidwarshawgarydgregory
davidwarshaw
authored and
garydgregory
committed
[CSV-224] Some Multi Iterator Parsing Peek Sequences Incorrectly Consume
Elements.
1 parent 33f662b commit f368f64

3 files changed

Lines changed: 106 additions & 44 deletions

File tree

‎src/changes/changes.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
<action issue="CSV-220" type="add" dev="ggregory" due-to="Gary Gregory">Add API org.apache.commons.csv.CSVFormat.withSystemRecordSeparator().</action>
4646
<action issue="CSV-223" type="fix" dev="ggregory" due-to="Samuel Martin">Inconsistency between Javadoc of CSVFormat DEFAULT EXCEL.</action>
4747
<action issue="CSV-209" type="fix" dev="ggregory" due-to="Gary Gregory">Create CSVFormat.ORACLE preset.</action>
48+
<action issue="CSV-224" type="fix" dev="ggregory" due-to="David Warshaw">Some Multi Iterator Parsing Peek Sequences Incorrectly Consume Elements.</action>
4849
</release>
4950
<release version="1.5" date="2017-09-03" description="Feature and bug fix release">
5051
<action issue="CSV-203" type="fix" dev="ggregory" due-to="Richard Wheeldon, Kai Paroth">withNullString value is printed without quotes when QuoteMode.ALL is specified; add QuoteMode.ALL_NON_NULL. PR #17.</action>

‎src/main/java/org/apache/commons/csv/CSVParser.java‎

Lines changed: 49 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,8 @@ public static CSVParser parse(final URL url, final Charset charset, final CSVFor
286286

287287
private final Lexer lexer;
288288

289+
private final CSVRecordIterator csvRecordIterator;
290+
289291
/** A record buffer for getRecord(). Grows as necessary and is reused. */
290292
private final List<String> recordList = new ArrayList<>();
291293

@@ -353,6 +355,7 @@ public CSVParser(final Reader reader, final CSVFormat format, final long charact
353355

354356
this.format = format;
355357
this.lexer = new Lexer(format, new ExtendedBufferedReader(reader));
358+
this.csvRecordIterator = new CSVRecordIterator();
356359
this.headerMap = this.initializeHeader();
357360
this.characterOffset = characterOffset;
358361
this.recordNumber = recordNumber - 1;
@@ -519,55 +522,57 @@ public boolean isClosed() {
519522
*/
520523
@Override
521524
public Iterator<CSVRecord> iterator() {
522-
return new Iterator<CSVRecord>() {
523-
private CSVRecord current;
524-
525-
private CSVRecord getNextRecord() {
526-
try {
527-
return CSVParser.this.nextRecord();
528-
} catch (final IOException e) {
529-
throw new IllegalStateException(
530-
e.getClass().getSimpleName() + " reading next record: " + e.toString(), e);
531-
}
525+
return csvRecordIterator;
526+
}
527+
528+
class CSVRecordIterator implements Iterator<CSVRecord> {
529+
private CSVRecord current;
530+
531+
private CSVRecord getNextRecord() {
532+
try {
533+
return CSVParser.this.nextRecord();
534+
} catch (final IOException e) {
535+
throw new IllegalStateException(
536+
e.getClass().getSimpleName() + " reading next record: " + e.toString(), e);
532537
}
533-
534-
@Override
535-
public boolean hasNext() {
536-
if (CSVParser.this.isClosed()) {
537-
return false;
538-
}
539-
if (this.current == null) {
540-
this.current = this.getNextRecord();
541-
}
542-
543-
return this.current != null;
538+
}
539+
540+
@Override
541+
public boolean hasNext() {
542+
if (CSVParser.this.isClosed()) {
543+
return false;
544544
}
545-
546-
@Override
547-
public CSVRecord next() {
548-
if (CSVParser.this.isClosed()) {
549-
throw new NoSuchElementException("CSVParser has been closed");
550-
}
551-
CSVRecord next = this.current;
552-
this.current = null;
553-
545+
if (this.current == null) {
546+
this.current = this.getNextRecord();
547+
}
548+
549+
return this.current != null;
550+
}
551+
552+
@Override
553+
public CSVRecord next() {
554+
if (CSVParser.this.isClosed()) {
555+
throw new NoSuchElementException("CSVParser has been closed");
556+
}
557+
CSVRecord next = this.current;
558+
this.current = null;
559+
560+
if (next == null) {
561+
// hasNext() wasn't called before
562+
next = this.getNextRecord();
554563
if (next == null) {
555-
// hasNext() wasn't called before
556-
next = this.getNextRecord();
557-
if (next == null) {
558-
throw new NoSuchElementException("No more CSV records available");
559-
}
564+
throw new NoSuchElementException("No more CSV records available");
560565
}
561-
562-
return next;
563566
}
564-
565-
@Override
566-
public void remove() {
567-
throw new UnsupportedOperationException();
568-
}
569-
};
570-
}
567+
568+
return next;
569+
}
570+
571+
@Override
572+
public void remove() {
573+
throw new UnsupportedOperationException();
574+
}
575+
};
571576

572577
/**
573578
* Parses the next record from the current point in the stream.

‎src/test/java/org/apache/commons/csv/CSVParserTest.java‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -989,6 +989,62 @@ public void testTrim() throws Exception {
989989
Assert.assertEquals(3, record.size());
990990
}
991991

992+
@Test
993+
public void testIteratorSequenceBreaking() throws IOException {
994+
final String fiveRows = "1\n2\n3\n4\n5\n";
995+
996+
// Iterator hasNext() shouldn't break sequence
997+
CSVParser parser = CSVFormat.DEFAULT.parse(new StringReader(fiveRows));
998+
int recordNumber = 0;
999+
Iterator<CSVRecord> iter = parser.iterator();
1000+
recordNumber = 0;
1001+
while (iter.hasNext()) {
1002+
CSVRecord record = iter.next();
1003+
recordNumber++;
1004+
assertEquals(String.valueOf(recordNumber), record.get(0));
1005+
if (recordNumber >= 2) {
1006+
break;
1007+
}
1008+
}
1009+
iter.hasNext();
1010+
while (iter.hasNext()) {
1011+
CSVRecord record = iter.next();
1012+
recordNumber++;
1013+
assertEquals(String.valueOf(recordNumber), record.get(0));
1014+
}
1015+
1016+
// Consecutive enhanced for loops shouldn't break sequence
1017+
parser = CSVFormat.DEFAULT.parse(new StringReader(fiveRows));
1018+
recordNumber = 0;
1019+
for (CSVRecord record : parser) {
1020+
recordNumber++;
1021+
assertEquals(String.valueOf(recordNumber), record.get(0));
1022+
if (recordNumber >= 2) {
1023+
break;
1024+
}
1025+
}
1026+
for (CSVRecord record : parser) {
1027+
recordNumber++;
1028+
assertEquals(String.valueOf(recordNumber), record.get(0));
1029+
}
1030+
1031+
// Consecutive enhanced for loops with hasNext() peeking shouldn't break sequence
1032+
parser = CSVFormat.DEFAULT.parse(new StringReader(fiveRows));
1033+
recordNumber = 0;
1034+
for (CSVRecord record : parser) {
1035+
recordNumber++;
1036+
assertEquals(String.valueOf(recordNumber), record.get(0));
1037+
if (recordNumber >= 2) {
1038+
break;
1039+
}
1040+
}
1041+
parser.iterator().hasNext();
1042+
for (CSVRecord record : parser) {
1043+
recordNumber++;
1044+
assertEquals(String.valueOf(recordNumber), record.get(0));
1045+
}
1046+
}
1047 4C65 +
9921048
private void validateLineNumbers(final String lineSeparator) throws IOException {
9931049
try (final CSVParser parser = CSVParser.parse("a" + lineSeparator + "b" + lineSeparator + "c",
9941050
CSVFormat.DEFAULT.withRecordSeparator(lineSeparator))) {

0 commit comments

Comments
 (0)