Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions csv_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,13 @@ CSVParseResult csv_parse_line_inplace(const char *line, Arena *arena, const CSVC
case FIELD_START:
if (c == config->enclosure) {
state = QUOTED_FIELD;
field_start = &line[pos + 1];
field_len = 0;
if (!config->preserveQuotes) {
field_start = &line[pos + 1];
field_len = 0;
} else {
field_start = &line[pos];
field_len = 1;
}
Comment on lines +122 to +128

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy lift

Boundary quotes get mis-collapsed by add_quoted_field's escape logic when preserveQuotes is enabled.

Including the opening/closing enclosure chars directly in field_start/field_len means the span later passed to add_quoted_field (lines 179, 214) now contains the boundary quote characters. add_quoted_field's existing escape-collapsing loop (lines 75-82) treats any two consecutive enclosure characters as an escaped pair and collapses them to one — it has no way to distinguish "real" escaped quotes from the artificially-included boundary quotes.

Concrete repro: parsing "",normal with preserveQuotes = true:

  • field_start = &line[0], field_len = 2 (span = "").
  • add_quoted_field sees start[0]=='"' and start[1]=='"' and collapses them into a single ".
  • Result: the empty quoted field becomes " instead of the expected "".

The same mis-collapse happens whenever content immediately adjacent to a boundary quote is itself an escaped quote (e.g. """quoted"""), producing output that is neither the verbatim source nor correctly re-quoted content.

Since add_quoted_field itself wasn't updated for this PR, this is a latent correctness bug not caught by the added test (which uses a quoted field with no adjacent/internal quotes).

🛠 Suggested fix — don't feed boundary quotes into the escape-scan; wrap after unescaping

Keep field_start/field_len scoped to content only (as before), and instead have add_quoted_field write the enclosure chars directly, outside the escape-scanning loop:

-static bool add_quoted_field(FieldArray *arr, const char *start, size_t len, Arena *arena, char enclosure) {
+static bool add_quoted_field(FieldArray *arr, const char *start, size_t len, Arena *arena, char enclosure, bool preserve_quotes) {
     if (arr->count >= arr->capacity) {
         if (!grow_field_array(arr, arena)) {
             return false;
         }
     }
 
     void *ptr;
-    ArenaResult result = arena_alloc(arena, len + 1, &ptr);
+    ArenaResult result = arena_alloc(arena, len + 3, &ptr);
     if (result != ARENA_OK) {
         return false;
     }
     
     char *field = (char*)ptr;
     size_t write_pos = 0;
+    if (preserve_quotes) field[write_pos++] = enclosure;
     
     for (size_t i = 0; i < len; i++) {
         if (start[i] == enclosure && i + 1 < len && start[i + 1] == enclosure) {
             field[write_pos++] = enclosure;
             i++;
         } else {
             field[write_pos++] = start[i];
         }
     }
-    
+    if (preserve_quotes) field[write_pos++] = enclosure;
     field[write_pos] = '\0';

Then revert the state-machine changes to always exclude the boundary quotes from field_start/field_len:

case FIELD_START:
    if (c == config->enclosure) {
        state = QUOTED_FIELD;
        field_start = &line[pos + 1];
        field_len = 0;
    }
case QUOTED_FIELD:
    if (c == config->enclosure) {
        if (pos + 1 < len && line[pos + 1] == config->enclosure) {
            field_len += 2;
            pos++;
        } else {
            state = FIELD_END;
        }
    }

And pass config->preserveQuotes at both call sites (lines 179 and 214):

add_quoted_field(&result.fields, field_start, field_len, arena, config->enclosure, config->preserveQuotes)

Also applies to: 167-169

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@csv_parser.c` around lines 122 - 128, The preserveQuotes handling in the
quoted-field path is feeding boundary enclosure chars into add_quoted_field,
which then mis-collapses them as escaped quotes. Keep field_start/field_len
pointed only at the inner content in the FIELD_START/QUOTED_FIELD logic, and let
add_quoted_field wrap the enclosure chars itself outside its escape-collapsing
loop. Update both call sites that invoke add_quoted_field to pass preserveQuotes
so it can decide whether to re-add the boundary quotes without scanning them as
input.

} else if (c == config->delimiter) {
if (!add_field(&result.fields, "", 0, arena)) {
result.success = false;
Expand Down Expand Up @@ -159,6 +164,9 @@ CSVParseResult csv_parse_line_inplace(const char *line, Arena *arena, const CSVC
field_len += 2;
pos++;
} else {
if (config->preserveQuotes) {
field_len++;
}
state = FIELD_END;
}
} else {
Expand Down
8 changes: 8 additions & 0 deletions tests/test_csv_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ void test_csv_parser_escaped_quotes() {
assert(result2.fields.count == 2);
assert(strcmp(result2.fields.fields[0], "\"quoted\"") == 0);
assert(strcmp(result2.fields.fields[1], "test") == 0);

printf("Testing CSV parser with preserveQuotes=true...\n");
config->preserveQuotes = true;
CSVParseResult result3 = csv_parse_line_inplace("\"Say Hello World\",normal", &arena, config, 3);
assert(result3.success == true);
assert(result3.fields.count == 2);
assert(strcmp(result3.fields.fields[0], "\"Say Hello World\"") == 0);
assert(strcmp(result3.fields.fields[1], "normal") == 0);

arena_destroy(&arena);
printf("✓ CSV parser escaped quotes test passed\n");
Expand Down