From 4e4562e6227b3c487e36899189546d7135ef116f Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 03:35:19 +0200 Subject: [PATCH 1/2] fix(apple): validate CSV delimiters and root interfaces --- CHANGELOG.md | 3 +++ apple/include/OdrCoreObjC/ODRFile.h | 4 ++-- apple/src/ODRDocumentElement.mm | 4 +++- apple/src/ODRFile.mm | 21 ++++++++++++++------- apple/tests/OdrCoreTests.swift | 22 +++++++++++++++++++++- 5 files changed, 43 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea122f9ff..9ebf93f8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- Apple CSV decoding rejects empty, multi-character and multibyte delimiters. + Roots without a text-root interface are exposed as `Element`, not `TextRoot`. + - Apple `Measure(string:)` is now failable. An invalid length returns `nil` instead of letting a C++ exception escape into Swift. diff --git a/apple/include/OdrCoreObjC/ODRFile.h b/apple/include/OdrCoreObjC/ODRFile.h index fbfb6481d..609260a47 100644 --- a/apple/include/OdrCoreObjC/ODRFile.h +++ b/apple/include/OdrCoreObjC/ODRFile.h @@ -228,9 +228,9 @@ NS_SWIFT_NAME(CsvOptions) @interface ODRCsvOptions : NSObject /// `nil` to detect. @property(nonatomic, strong, nullable) NSNumber *encoding; -/// A one-character string, `nil` to detect. +/// One UTF-8 byte (ASCII), `nil` to detect. Other strings fail decoding. @property(nonatomic, copy, nullable) NSString *separator; -/// A one-character string, `nil` to detect. +/// One UTF-8 byte (ASCII), `nil` to detect. Other strings fail decoding. @property(nonatomic, copy, nullable) NSString *quote; @end diff --git a/apple/src/ODRDocumentElement.mm b/apple/src/ODRDocumentElement.mm index 97363e3ed..278770352 100644 --- a/apple/src/ODRDocumentElement.mm +++ b/apple/src/ODRDocumentElement.mm @@ -102,7 +102,9 @@ + (nullable ODRElement *)elementWithHandle:(odr::Element)handle Class klass = [ODRElement class]; switch (handle.type()) { case odr::ElementType::root: - klass = [ODRTextRoot class]; + if (handle.as_text_root()) { + klass = [ODRTextRoot class]; + } break; case odr::ElementType::slide: klass = [ODRSlide class]; diff --git a/apple/src/ODRFile.mm b/apple/src/ODRFile.mm index 507adafcc..6252bfe80 100644 --- a/apple/src/ODRFile.mm +++ b/apple/src/ODRFile.mm @@ -8,6 +8,7 @@ #include #include +#include #include using odr::apple::guarded; @@ -143,6 +144,17 @@ return result; } +std::optional csv_delimiter(NSString *const value) { + if (value == nil) { + return std::nullopt; + } + const std::string bytes = to_string(value); + if (bytes.size() != 1) { + throw std::invalid_argument("CSV delimiter must be one UTF-8 byte"); + } + return bytes.front(); +} + /// `nil` for an unset `std::optional`, the way a Java binding would use -1. NSString *_Nullable to_nsstring(const std::optional &value) { return value.has_value() ? to_nsstring(*value) : nil; @@ -370,13 +382,8 @@ + (nullable instancetype)decodePath:(NSString *)path native.csv.encoding = static_cast(options.csv.encoding.integerValue); } - // one character, and an empty string means unset rather than a NUL - if (options.csv.separator.length > 0) { - native.csv.separator = [options.csv.separator characterAtIndex:0]; - } - if (options.csv.quote.length > 0) { - native.csv.quote = [options.csv.quote characterAtIndex:0]; - } + native.csv.separator = csv_delimiter(options.csv.separator); + native.csv.quote = csv_delimiter(options.csv.quote); return [ODRDecodedFile decodedFileWithHandle:odr::open(to_string(path), native)]; }); diff --git a/apple/tests/OdrCoreTests.swift b/apple/tests/OdrCoreTests.swift index b844f78ac..5400e80d4 100644 --- a/apple/tests/OdrCoreTests.swift +++ b/apple/tests/OdrCoreTests.swift @@ -118,7 +118,27 @@ final class DecodeTests: XCTestCase { let csv = try decoded.asCsvFile() XCTAssertEqual(try csv.textFile().text(), "a,b\n1,2\n") - XCTAssertNotNil(try csv.document().rootElement()) + let root = try XCTUnwrap(try csv.document().rootElement()) + XCTAssertFalse(root is TextRoot) + XCTAssertNotNil(root.firstDescendant(ofType: Sheet.self)) + } + + func testCsvDelimitersMustBeSingleBytes() throws { + let path = try write("a;b\nc;d\n", as: "table.csv") + let options = DecodeOptions() + options.csv.separator = ";" + let csv = try DecodedFile.decode(path: path, options: options).asCsvFile() + let root = try XCTUnwrap(try csv.document().rootElement()) + let sheet = try XCTUnwrap(root.firstDescendant(ofType: Sheet.self)) + XCTAssertEqual(sheet.dimensions.columns, 2) + for invalid in ["", ";,", "é", "😀"] { + options.csv.separator = invalid + XCTAssertThrowsError(try DecodedFile.decode(path: path, options: options), invalid) + options.csv.separator = ";" + options.csv.quote = invalid + XCTAssertThrowsError(try DecodedFile.decode(path: path, options: options), invalid) + options.csv.quote = nil + } } /// `odr::Filesystem::exists("")` throws `std::invalid_argument`. Unguarded, From 9e9803be69c71bbcbf1a91c092d37bc170dfeb49 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 23:07:57 +0200 Subject: [PATCH 2/2] fix(apple): keep an empty CSV delimiter as unset An empty separator or quote meant detect before, as the removed comment said; a cleared text field still does. Only a string of more than one UTF-8 byte fails decoding. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Gb1fLafqqzehpqPuU6uBfn --- CHANGELOG.md | 2 +- apple/include/OdrCoreObjC/ODRFile.h | 4 ++-- apple/src/ODRFile.mm | 3 ++- apple/tests/OdrCoreTests.swift | 4 +++- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ebf93f8c..095f88bef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ The release run heads these entries with the version and opens a fresh ## Unreleased -- Apple CSV decoding rejects empty, multi-character and multibyte delimiters. +- Apple CSV decoding rejects multi-character and multibyte delimiters. Roots without a text-root interface are exposed as `Element`, not `TextRoot`. - Apple `Measure(string:)` is now failable. An invalid length returns `nil` diff --git a/apple/include/OdrCoreObjC/ODRFile.h b/apple/include/OdrCoreObjC/ODRFile.h index 609260a47..307872c06 100644 --- a/apple/include/OdrCoreObjC/ODRFile.h +++ b/apple/include/OdrCoreObjC/ODRFile.h @@ -228,9 +228,9 @@ NS_SWIFT_NAME(CsvOptions) @interface ODRCsvOptions : NSObject /// `nil` to detect. @property(nonatomic, strong, nullable) NSNumber *encoding; -/// One UTF-8 byte (ASCII), `nil` to detect. Other strings fail decoding. +/// One ASCII character, `nil` or empty to detect. Other strings fail decoding. @property(nonatomic, copy, nullable) NSString *separator; -/// One UTF-8 byte (ASCII), `nil` to detect. Other strings fail decoding. +/// One ASCII character, `nil` or empty to detect. Other strings fail decoding. @property(nonatomic, copy, nullable) NSString *quote; @end diff --git a/apple/src/ODRFile.mm b/apple/src/ODRFile.mm index 6252bfe80..a68434f3b 100644 --- a/apple/src/ODRFile.mm +++ b/apple/src/ODRFile.mm @@ -145,7 +145,8 @@ } std::optional csv_delimiter(NSString *const value) { - if (value == nil) { + // an empty string means unset rather than a NUL + if (value.length == 0) { return std::nullopt; } const std::string bytes = to_string(value); diff --git a/apple/tests/OdrCoreTests.swift b/apple/tests/OdrCoreTests.swift index 5400e80d4..06c2710de 100644 --- a/apple/tests/OdrCoreTests.swift +++ b/apple/tests/OdrCoreTests.swift @@ -131,7 +131,9 @@ final class DecodeTests: XCTestCase { let root = try XCTUnwrap(try csv.document().rootElement()) let sheet = try XCTUnwrap(root.firstDescendant(ofType: Sheet.self)) XCTAssertEqual(sheet.dimensions.columns, 2) - for invalid in ["", ";,", "é", "😀"] { + options.csv.quote = "" + XCTAssertNoThrow(try DecodedFile.decode(path: path, options: options)) + for invalid in [";,", "é", "😀"] { options.csv.separator = invalid XCTAssertThrowsError(try DecodedFile.decode(path: path, options: options), invalid) options.csv.separator = ";"