diff --git a/CHANGELOG.md b/CHANGELOG.md index ea122f9ff..095f88bef 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 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..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; -/// A one-character string, `nil` to detect. +/// One ASCII character, `nil` or empty to detect. Other strings fail decoding. @property(nonatomic, copy, nullable) NSString *separator; -/// A one-character string, `nil` to detect. +/// One ASCII character, `nil` or empty 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..a68434f3b 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,18 @@ return result; } +std::optional csv_delimiter(NSString *const value) { + // an empty string means unset rather than a NUL + if (value.length == 0) { + 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 +383,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..06c2710de 100644 --- a/apple/tests/OdrCoreTests.swift +++ b/apple/tests/OdrCoreTests.swift @@ -118,7 +118,29 @@ 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) + 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 = ";" + 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,