Skip to content

Commit 660e34a

Browse files
committed
Harden parser lifetime, EOF handling, and validation tooling
Keep the existing streaming C++11 parser API and permissive CSV behavior while tightening correctness and production validation. Parser changes:\n- Fix EOF handling for trailing empty fields, empty quoted fields, and empty fields at EOF.\n- Fix position() accounting so it tracks consumed parser position instead of buffered chunk size.\n- Add an owning std::unique_ptr<std::istream> constructor and CsvParser::from_file(path).\n- Preserve the borrowed std::istream& constructor for compatibility.\n- Reject null owned streams and failed input streams early.\n- Improve common payload parsing by appending contiguous field spans instead of pushing every byte individually. Validation and release tooling:\n- Add regression tests for EOF, position, and stream ownership behavior.\n- Add RapidCheck property tests.\n- Add AFL-style, libFuzzer, and deterministic sanitizer mutation fuzz harnesses.\n- Add GitHub Actions CI for tests, fuzz smoke builds, and benchmark harness checks.\n- Add security notes and benchmark/profiling workflow docs. Compatibility:\n- Existing CsvParser(std::istream&) usage continues to work.\n- Default parsing remains permissive.\n- No strict mode or size limits are enabled by default.
1 parent 43961a9 commit 660e34a

24 files changed

Lines changed: 1819 additions & 49 deletions

.github/workflows/ci.yml

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
name: CI
2+
3+
on:
4+
push:
5+
pull_request:
6+
7+
jobs:
8+
test:
9+
runs-on: ubuntu-latest
10+
steps:
11+
- uses: actions/checkout@v4
12+
13+
- name: Configure unit tests
14+
run: cmake -S test -B test/out
15+
16+
- name: Build unit tests
17+
run: cmake --build test/out --parallel
18+
19+
- name: Run unit tests
20+
run: ./test/out/parser_test
21+
22+
- name: Configure property tests
23+
run: cmake -S test -B test/property-out -DARIA_CSV_ENABLE_PROPERTY_TESTS=ON
24+
25+
- name: Build property tests
26+
run: cmake --build test/property-out --parallel
27+
28+
- name: Run property tests
29+
run: ./test/property-out/parser_property_test
30+
31+
fuzz-smoke:
32+
runs-on: ubuntu-latest
33+
steps:
34+
- uses: actions/checkout@v4
35+
36+
- name: Build AFL-compatible harness
37+
run: clang++ -std=c++11 -Wall -Wextra -pedantic -I. fuzz/afl_parser.cpp -o /tmp/aria_csv_afl_parser
38+
39+
- name: Build libFuzzer harness
40+
run: clang++ -std=c++11 -Wall -Wextra -pedantic -I. -fsanitize=fuzzer,address,undefined fuzz/libfuzzer_parser.cpp -o /tmp/aria_csv_libfuzzer
41+
42+
- name: Configure CMake fuzz targets
43+
run: cmake -S . -B /tmp/aria_csv_fuzz_build -DARIA_CSV_BUILD_FUZZERS=ON
44+
45+
- name: Build CMake AFL-compatible fuzz target
46+
run: cmake --build /tmp/aria_csv_fuzz_build --target aria_csv_afl_fuzzer --parallel
47+
48+
- name: Run libFuzzer smoke test
49+
run: |
50+
mkdir -p /tmp/aria_csv_corpus
51+
cp test/data/*.csv /tmp/aria_csv_corpus/
52+
/tmp/aria_csv_libfuzzer /tmp/aria_csv_corpus -runs=1000
53+
54+
build:
55+
runs-on: ubuntu-latest
56+
steps:
57+
- uses: actions/checkout@v4
58+
59+
- name: Build benchmark harnesses
60+
run: |
61+
clang++ -std=c++11 -Wall -Wextra -pedantic -I. -c benchmark/cpp/main.cpp -o /tmp/aria_csv_benchmark.o
62+
clang++ -std=c++11 -Wall -Wextra -pedantic -I. -c benchmark/profile.cpp -o /tmp/aria_csv_profile.o
63+
clang++ -std=c++11 -Wall -Wextra -pedantic -I. -c benchmark/optimize.cpp -o /tmp/aria_csv_optimize.o
64+
65+
- name: Run optimization harness smoke test
66+
run: |
67+
clang++ -std=c++11 -O2 -DNDEBUG -I. benchmark/optimize.cpp -o /tmp/aria_csv_optimize
68+
/tmp/aria_csv_optimize 1 | tee /tmp/aria_csv_optimize_a.csv
69+
/tmp/aria_csv_optimize 1 | tee /tmp/aria_csv_optimize_b.csv
70+
python3 benchmark/compare_results.py /tmp/aria_csv_optimize_a.csv /tmp/aria_csv_optimize_b.csv

ARCHITECTURE.md

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
# Architecture
2+
3+
`CsvParser` is a small streaming parser. It reads bytes from a `std::istream`,
4+
keeps a fixed-size input buffer, and exposes parsed data through `next_field()`
5+
or the row iterator.
6+
7+
## Main Pieces
8+
9+
```text
10+
+----------------------+ +----------------------+ +----------------------+
11+
| User code | | CsvParser | | std::istream |
12+
|----------------------| |----------------------| |----------------------|
13+
| next_field() | --> | parser state machine | --> | file/string/stdin |
14+
| for (row : parser) | | input buffer | | borrowed or owned |
15+
| position() | | field buffer | | |
16+
+----------------------+ +----------------------+ +----------------------+
17+
```
18+
19+
The parser can either borrow a stream or own one:
20+
21+
```text
22+
+----------------------+ +----------------------+
23+
| Borrowed stream | | Owned stream |
24+
|----------------------| |----------------------|
25+
| CsvParser(istream&) | | CsvParser(unique_ptr)|
26+
| caller owns stream | | parser owns stream |
27+
| caller manages life | | safe for stored use |
28+
+----------------------+ +----------------------+
29+
```
30+
31+
`CsvParser::from_file(path)` is a convenience wrapper around the owned-stream
32+
case.
33+
34+
## Data Flow
35+
36+
```text
37+
+--------------+ +--------------+ +--------------+ +--------------+
38+
| input stream | --> | input buffer | --> | next_field() | --> | Field |
39+
| read() | | 128 KiB | | CSV states | | DATA/ROW/END |
40+
+--------------+ +--------------+ +--------------+ +--------------+
41+
| |
42+
| v
43+
| +--------------+
44+
+----------> | field buffer |
45+
| std::string |
46+
+--------------+
47+
```
48+
49+
The row iterator is built on top of `next_field()`:
50+
51+
```text
52+
+--------------+ +--------------+ +----------------------+
53+
| next_field() | --> | iterator | --> | vector<string> row |
54+
| field events | | row builder | | reused between rows |
55+
+--------------+ +--------------+ +----------------------+
56+
```
57+
58+
## Parser States
59+
60+
```text
61+
+----------------+ quote +-------------------+
62+
| START_OF_FIELD | -----------------> | IN_QUOTED_FIELD |
63+
+----------------+ +-------------------+
64+
| |
65+
| normal char | quote
66+
v v
67+
+----------------+ delimiter +-------------------+
68+
| IN_FIELD | -----------------> | IN_ESCAPED_QUOTE |
69+
+----------------+ +-------------------+
70+
| |
71+
| newline | delimiter/newline
72+
v v
73+
+----------------+ +-------------------+
74+
| END_OF_ROW | -----------------> | START_OF_FIELD |
75+
+----------------+ +-------------------+
76+
```
77+
78+
`EMPTY` is the terminal state after CSV end.
79+
80+
## Field Scanning
81+
82+
The state machine still owns the CSV rules. The scanner has a small optimization
83+
for ordinary field text: once it knows it is inside an unquoted or quoted field,
84+
it copies contiguous field characters as a range until the next structural
85+
character.
86+
87+
```text
88+
Unquoted field:
89+
90+
abc,def
91+
^^^
92+
copy chars until comma or row terminator
93+
94+
Quoted field:
95+
96+
"abc,def"
97+
^^^^^^^
98+
copy chars until quote
99+
```
100+
101+
That is what the helper names mean:
102+
103+
```text
104+
has_unquoted_field_chars() more ordinary unquoted bytes are buffered
105+
append_unquoted_field_chars() append them to the current field
106+
has_quoted_field_chars() more quoted bytes are buffered
107+
append_quoted_field_chars() append them to the current field
108+
```
109+
110+
The optimization does not change the public API. It only avoids appending one
111+
character at a time when the next several bytes are known to be field contents.
112+
113+
## Empty Lines
114+
115+
An empty line is a row boundary, not an empty data field.
116+
117+
```text
118+
Input bytes: \n\n
119+
next_field(): ROW_END
120+
next_field(): ROW_END
121+
next_field(): CSV_END
122+
```
123+
124+
An empty field still returns `DATA`:
125+
126+
```text
127+
Input bytes: ,
128+
next_field(): DATA("")
129+
next_field(): DATA("")
130+
next_field(): CSV_END
131+
```

CMakeLists.txt

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1-
project(AriaCsvParser)
21
cmake_minimum_required(VERSION 3.14 FATAL_ERROR)
2+
project(AriaCsvParser)
33
include(GNUInstallDirs)
44
include(CMakePackageConfigHelpers)
55

6+
option(ARIA_CSV_BUILD_FUZZERS "Build parser fuzz targets" OFF)
7+
option(ARIA_CSV_BUILD_LIBFUZZER "Build the libFuzzer parser target" OFF)
8+
69
add_library(${PROJECT_NAME} INTERFACE)
710

811
set_target_properties(${PROJECT_NAME} PROPERTIES
@@ -16,7 +19,8 @@ target_include_directories( ${PROJECT_NAME}
1619
)
1720

1821
install(TARGETS ${PROJECT_NAME}
19-
EXPORT ${PROJECT_NAME}Targets)
22+
EXPORT ${PROJECT_NAME}Targets
23+
PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
2024

2125
install(EXPORT ${PROJECT_NAME}Targets
2226
NAMESPACE AriaCsvParser::
@@ -34,4 +38,8 @@ configure_package_config_file(Config.cmake.in
3438
install(FILES
3539
${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}Config.cmake
3640
DESTINATION lib/cmake/${PROJECT_NAME}
37-
)
41+
)
42+
43+
if(ARIA_CSV_BUILD_FUZZERS)
44+
add_subdirectory(fuzz)
45+
endif()

README.md

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,23 @@ or you can read from `stdin`
2020
CsvParser parser(std::cin);
2121
```
2222

23+
The parser can also own the input stream, which is safer when the parser needs
24+
to outlive the scope where the stream is created.
25+
26+
```cpp
27+
auto parser = CsvParser::from_file("some_file.csv");
28+
```
29+
30+
or
31+
32+
```cpp
33+
std::unique_ptr<std::istream> input(new std::ifstream("some_file.csv"));
34+
CsvParser parser(std::move(input));
35+
```
36+
37+
When using the `std::istream&` constructor, the caller must keep the stream alive
38+
for at least as long as the parser.
39+
2340
Moreover, you can configure the parser by chaining configuration methods like
2441
2542
```cpp
@@ -72,7 +89,7 @@ int main() {
7289
auto field = parser.next_field();
7390
switch (field.type) {
7491
case FieldType::DATA:
75-
std::cout << *field.data << " | ";
92+
std::cout << field.data << " | ";
7693
break;
7794
case FieldType::ROW_END:
7895
std::cout << std::endl;
@@ -92,4 +109,21 @@ reporting things like progress through a file. You can use
92109

93110
## Testing
94111

95-
Run `cmake -B out && cmake --build out && ./out/parser_test` in test dir
112+
Run the unit tests with:
113+
114+
```sh
115+
cmake -S test -B test/out
116+
cmake --build test/out
117+
./test/out/parser_test
118+
```
119+
120+
Property tests are opt-in and use RapidCheck:
121+
122+
```sh
123+
cmake -S test -B test/out -DARIA_CSV_ENABLE_PROPERTY_TESTS=ON
124+
cmake --build test/out
125+
./test/out/parser_property_test
126+
```
127+
128+
Fuzz targets live in `fuzz/`. See `fuzz/README.md` for libFuzzer and AFL++
129+
commands.

0 commit comments

Comments
 (0)