Skip to content

CSVExport.escaped misses CRLF: Swift treats "\r\n" as one Character equal to neither \n nor \r #55

Description

@evandhoffman

Sources/MonitorCore/CSVExport.swift:

/// RFC 4180: a field holding a comma, a quote or a newline is wrapped in
/// quotes, and its own quotes are doubled.
static func escaped(_ field: String) -> String {
    guard field.contains(where: { $0 == "," || $0 == "\"" || $0 == "\n" || $0 == "\r" })
    else { return field }
    return "\"\(field.replacingOccurrences(of: "\"", with: "\"\""))\""
}

The predicate iterates Character, and in Swift "\r\n" is a single
grapheme cluster that compares equal to neither "\n" nor "\r". So a field
containing a Windows line ending passes the guard untouched and is written
unquoted, breaking the row for any RFC 4180 reader.

The doc comment says "a newline", and CRLF is a newline; the code disagrees
with its own contract.

Not reachable today

Worth stating plainly so this is prioritized honestly: every field that reaches
escaped is currently code-controlled — metric names, formatted numbers, ISO
timestamps — except the hostname, and a hostname containing CRLF is not a
realistic input. This is a latent correctness bug, not a live one.

It becomes reachable the moment any externally-sourced string enters a CSV
field: a label, a note, a device name read from the system, or a user-supplied
tag. The fix is cheap enough now that it is not worth waiting for that.

Fix

Iterate scalars rather than characters, so CRLF is seen as the two scalars it
contains:

guard field.unicodeScalars.contains(where: { $0 == "," || $0 == "\"" || $0 == "\n" || $0 == "\r" })
else { return field }

A test with a field containing "a\r\nb" fails before this change and passes
after, which is the case worth pinning since it is the one that reads as
obviously-handled and is not.

Reported by an adjacent session that spotted it while reading the export path;
verified against origin/main.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions