diff --git a/android/src/androidTest/java/app/opendocument/core/DocumentTest.kt b/android/src/androidTest/java/app/opendocument/core/DocumentTest.kt index 79f9b873c..0bc5f8b91 100644 --- a/android/src/androidTest/java/app/opendocument/core/DocumentTest.kt +++ b/android/src/androidTest/java/app/opendocument/core/DocumentTest.kt @@ -94,8 +94,6 @@ class DocumentTest { val pages = html.pages() assertEquals(1, pages.size) - // the renderer reads the css/js the AAR ships, so this only passes with the - // extracted assets in place val content = read(Paths.get(pages[0].path)) assertTrue(content.contains(TestFiles.ODT_WORD)) } diff --git a/apple/include/OdrCoreObjC/ODRTable.h b/apple/include/OdrCoreObjC/ODRTable.h index a0c40af6f..0434580dd 100644 --- a/apple/include/OdrCoreObjC/ODRTable.h +++ b/apple/include/OdrCoreObjC/ODRTable.h @@ -35,8 +35,11 @@ NS_INLINE ODRTablePosition ODRTablePositionMake(uint32_t column, uint32_t row) { NS_SWIFT_NAME(TableAddress) @interface ODRTableAddress : NSObject +/// 0 for anything that is not a column, e.g. `"c"` or `""`; use +/// `position:fromString:` to be told why instead. + (uint32_t)columnNumberFromString:(NSString *)string NS_SWIFT_NAME(columnNumber(from:)); +/// 0 for anything that is not a row number. + (uint32_t)rowNumberFromString:(NSString *)string NS_SWIFT_NAME(rowNumber(from:)); + (NSString *)stringFromColumnNumber:(uint32_t)column diff --git a/apple/src/ODRDocumentElement.mm b/apple/src/ODRDocumentElement.mm index cd899cf23..6894885f3 100644 --- a/apple/src/ODRDocumentElement.mm +++ b/apple/src/ODRDocumentElement.mm @@ -60,13 +60,24 @@ ODRTableDimensions to_dimensions(const odr::TableDimensions &value) { return ODRTableDimensionsMake(value.rows, value.columns); } +/// Wraps a range through `-derive:`, so `source`'s owner is carried along. +NSArray *to_nsarray(ODRElement *const source, + const odr::ElementRange &range) { + NSMutableArray *const result = [NSMutableArray array]; + for (const odr::Element element : range) { + if (ODRElement *const wrapped = [source derive:element]; wrapped != nil) { + [result addObject:wrapped]; + } + } + return result; +} + } // namespace @implementation ODRElement { odr::Element _handle; - // The document the adapter behind `_handle` belongs to. `odr::Element` holds - // a bare pointer into it, so without this the tree could outlive what it - // points into — the analogue of the JNI bindings' owner chain. + // `odr::Element` holds a bare pointer into the document's adapter, so the + // tree has to keep the document alive itself. id _owner; } @@ -212,15 +223,7 @@ - (nullable ODRElement *)nextSibling { - (NSArray *)children { return guarded_value( [&]() -> NSArray * { - NSMutableArray *const result = [NSMutableArray array]; - for (odr::Element child = _handle.first_child(); child; - child = child.next_sibling()) { - ODRElement *const wrapped = [self derive:child]; - if (wrapped != nil) { - [result addObject:wrapped]; - } - } - return result; + return to_nsarray(self, _handle.children()); }, @[]); } @@ -333,14 +336,7 @@ - (nullable ODRElement *)cellAtColumn:(uint32_t)column row:(uint32_t)row { - (NSArray *)shapes { return guarded_value( [&]() -> NSArray * { - NSMutableArray *const result = [NSMutableArray array]; - for (const odr::Element shape : self.handle.as_sheet().shapes()) { - ODRElement *const wrapped = [self derive:shape]; - if (wrapped != nil) { - [result addObject:wrapped]; - } - } - return result; + return to_nsarray(self, self.handle.as_sheet().shapes()); }, @[]); } @@ -578,14 +574,7 @@ - (nullable ODRElement *)firstColumn { - (NSArray *)columns { return guarded_value( [&]() -> NSArray * { - NSMutableArray *const result = [NSMutableArray array]; - for (const odr::Element column : self.handle.as_table().columns()) { - ODRElement *const wrapped = [self derive:column]; - if (wrapped != nil) { - [result addObject:wrapped]; - } - } - return result; + return to_nsarray(self, self.handle.as_table().columns()); }, @[]); } @@ -593,14 +582,7 @@ - (nullable ODRElement *)firstColumn { - (NSArray *)rows { return guarded_value( [&]() -> NSArray * { - NSMutableArray *const result = [NSMutableArray array]; - for (const odr::Element row : self.handle.as_table().rows()) { - ODRElement *const wrapped = [self derive:row]; - if (wrapped != nil) { - [result addObject:wrapped]; - } - } - return result; + return to_nsarray(self, self.handle.as_table().rows()); }, @[]); } diff --git a/apple/src/ODRFile.mm b/apple/src/ODRFile.mm index 35fd0dd27..7bdf264b9 100644 --- a/apple/src/ODRFile.mm +++ b/apple/src/ODRFile.mm @@ -271,8 +271,9 @@ @implementation ODRDecodedFile { + (instancetype)decodedFileWithHandle:(odr::DecodedFile)handle { // The most derived wrapper the file qualifies for, so a caller never has to - // downcast something it already knows the type of. PDFs are document files - // too, so they have to be tested first. + // downcast something it already knows the type of. The predicates are + // mutually exclusive — a PDF is not an `is_document_file`, despite reporting + // `FileCategory::document`. Class klass = [ODRDecodedFile class]; if (handle.is_pdf_file()) { klass = [ODRPdfFile class]; diff --git a/apple/src/ODRHtml.mm b/apple/src/ODRHtml.mm index d7a4ed867..692aff382 100644 --- a/apple/src/ODRHtml.mm +++ b/apple/src/ODRHtml.mm @@ -306,9 +306,8 @@ + (instancetype)htmlWithHandle:(const odr::Html &)handle { @implementation ODRHtmlView { std::optional _handle; - // The service the view belongs to. The view's impl holds a bare pointer to - // it, so without this a view handed out by `-views` could outlive what it - // points into — the same owner chain `ODRElement` keeps to its document. + // The view's impl holds a bare pointer to its service, so the view has to + // keep the service alive itself — as `ODRElement` does its document. id _owner; } diff --git a/apple/src/ODRInternal.h b/apple/src/ODRInternal.h index ad26aba09..ffe41e801 100644 --- a/apple/src/ODRInternal.h +++ b/apple/src/ODRInternal.h @@ -36,25 +36,13 @@ NSData *to_nsdata(std::istream &stream); /// inside a `catch`. void fill_error(NSError *_Nullable *_Nullable error); -/// Fills `*error` with `ODRErrorUnsupportedOperation` for an API area that is -/// declared but not bound yet, and returns nil. Every use is a placeholder to -/// delete, never a permanent answer. -id _Nullable not_yet_bound(NSError *_Nullable *_Nullable error, - const char *what); - /// Reports an exception that could not be handed to the caller. Call only from /// inside a `catch`. void report_swallowed(const char *what); /// Runs `body` where the caller has no way to receive an error — an ObjC -/// property, or a `void` method — and returns `fallback` if it throws. -/// -/// This is not politeness. An exception crossing into Objective-C++ unhandled -/// calls `std::terminate`, so an unguarded getter turns a malformed argument -/// into a crash of the *host app*; `odr::Filesystem::exists("")` throwing -/// `std::invalid_argument` is exactly how this was found. Almost nothing in -/// odrcore's public API is `noexcept`, so assume any call can throw and pick a -/// fallback that keeps the caller sane — `YES` for a walker's `end`, so a +/// property, or a `void` method — and returns `fallback` if it throws. Pick a +/// fallback that keeps the caller sane: `YES` for a walker's `end`, so a /// `while (!end)` loop terminates rather than spins. template auto guarded_value(Body &&body, std::invoke_result_t fallback) @@ -82,11 +70,8 @@ template void guarded_void(Body &&body) { } /// Runs `body`, mapping any C++ exception onto `*error`. A failed call returns -/// a value-initialised `Result` — `nil` for an object, `NO` for a `BOOL`, `0` -/// for a count — which is exactly the ObjC convention for "consult the error". -/// -/// Every binding body goes through this: an exception crossing into ObjC++ -/// unhandled would terminate the process. +/// a value-initialised `Result` — `nil`, `NO`, `0` — which is the ObjC +/// convention for "consult the error". template auto guarded(NSError *_Nullable *_Nullable error, Body &&body) -> decltype(body()) { diff --git a/apple/src/ODRLogger.mm b/apple/src/ODRLogger.mm index 72646d38a..49702267d 100644 --- a/apple/src/ODRLogger.mm +++ b/apple/src/ODRLogger.mm @@ -25,26 +25,33 @@ namespace { +void report_sink_exception(NSException *const exception) { + // A logger must not derail the operation it is reporting on. + NSLog(@"odr: log sink threw %@: %@", exception.name, exception.reason); +} + /// Routes `odr::ILogger` into an ObjC sink, the analogue of `jni_logger.cpp`'s -/// `JavaLogger`. -/// -/// Holds the sink strongly: the C++ logger can outlive every ObjC reference the -/// caller kept, and a sink collected out from under it would be a use after -/// free on a background thread. +/// `JavaLogger`. Holds the sink strongly: the C++ logger can outlive every ObjC +/// reference the caller kept. class SinkLogger final : public odr::ILogger { public: explicit SinkLogger(id sink) : m_sink{sink} {} [[nodiscard]] bool will_log(const odr::LogLevel level) const final { @autoreleasepool { - return [m_sink willLog:static_cast(level)] == YES; + @try { + return [m_sink willLog:static_cast(level)] == YES; + } @catch (NSException *const exception) { + report_sink_exception(exception); + return false; + } } } void log(const Time, const odr::LogLevel level, const std::string &message, const std::source_location &location) final { - // Log calls arrive on whatever thread the library works on, so each one - // gets its own pool rather than leaking into the caller's. + // Calls arrive on whatever thread the library works on, so each one gets + // its own pool rather than leaking into the caller's. @autoreleasepool { @try { [m_sink logLevel:static_cast(level) @@ -53,8 +60,7 @@ void log(const Time, const odr::LogLevel level, const std::string &message, std::string_view(location.file_name())) line:location.line()]; } @catch (NSException *const exception) { - // A logger must not derail the operation it is reporting on. - NSLog(@"odr: log sink threw %@: %@", exception.name, exception.reason); + report_sink_exception(exception); } } } @@ -64,7 +70,7 @@ void flush() final { @try { [m_sink flush]; } @catch (NSException *const exception) { - NSLog(@"odr: log sink threw %@: %@", exception.name, exception.reason); + report_sink_exception(exception); } } } diff --git a/apple/src/ODRPrivate.h b/apple/src/ODRPrivate.h index bfb4405b7..85affbc68 100644 --- a/apple/src/ODRPrivate.h +++ b/apple/src/ODRPrivate.h @@ -19,14 +19,10 @@ /// Cross-translation-unit access to the C++ value each wrapper owns. /// -/// The handle model is much lighter than the JNI one (`jni/AGENTS.md`): an -/// ObjC++ `@implementation` can hold the C++ handle as an ivar directly, and -/// ARC's `.cxx_construct`/`.cxx_destruct` run its constructor and destructor. -/// No `long` handles, no `destroy` natives, no reaper thread. -/// -/// Nor is there a keep-alive chain to maintain for these: the public C++ -/// handles hold a `shared_ptr` to the implementation, so a wrapper owning one -/// by value already keeps it alive on its own. +/// Each `@implementation` holds its handle as an ivar, destroyed by ARC's +/// `.cxx_destruct` — no `long` handles as in `jni/`. Most handles own a +/// `shared_ptr`, so a wrapper holding one needs no keep-alive; the exceptions +/// are `ODRElement` and `ODRHtmlView` below. /// /// Categories cannot add ivars, so each class declares its own accessors here /// and implements them next to its `@implementation`. diff --git a/apple/src/ODRStyle.mm b/apple/src/ODRStyle.mm index 955f4728a..28d687f34 100644 --- a/apple/src/ODRStyle.mm +++ b/apple/src/ODRStyle.mm @@ -7,6 +7,7 @@ #include +using odr::apple::guarded_value; using odr::apple::to_nsstring; ODR_SAME_ENUM(ODRFontWeightNormal, odr::FontWeight::normal); @@ -59,14 +60,11 @@ namespace { -/// The boxed forms of an absent `std::optional`. `nil` and not a sentinel: a -/// style that does not set a property is different from one that sets it to a -/// default, and only the caller knows what to fall back to. -NSNumber *_Nullable box_bool(const std::optional &value) { - return value.has_value() ? @(*value) : nil; -} - -NSNumber *_Nullable box_double(const std::optional &value) { +/// An absent `std::optional` boxes as `nil` and not as a sentinel: a style that +/// does not set a property is different from one that sets it to a default, and +/// only the caller knows what to fall back to. +template +NSNumber *_Nullable box_number(const std::optional &value) { return value.has_value() ? @(*value) : nil; } @@ -75,15 +73,10 @@ return value.has_value() ? @(static_cast(*value)) : nil; } -NSString *_Nullable box_string(const std::optional &value) { - return value.has_value() ? to_nsstring(*value) : nil; -} - -/// `font_name` is a `string_view` borrowing from the document that produced the -/// style, so it must be copied here — an `NSString` outliving that document is -/// the whole point of handing it to a caller. -NSString *_Nullable box_string_view( - const std::optional &value) { +/// Also takes the `string_view` of `font_name`, which borrows from the document +/// that produced the style — copying it here is the point. +template +NSString *_Nullable box_string(const std::optional &value) { return value.has_value() ? to_nsstring(*value) : nil; } @@ -102,15 +95,16 @@ + (instancetype)measureWithHandle:(const odr::Measure &)handle { } - (double)magnitude { - return _handle->magnitude(); + return guarded_value([&] { return _handle->magnitude(); }, 0.0); } - (NSString *)unit { - return to_nsstring(_handle->unit().name()); + return guarded_value([&] { return to_nsstring(_handle->unit().name()); }, + @""); } - (NSString *)stringValue { - return to_nsstring(_handle->to_string()); + return guarded_value([&] { return to_nsstring(_handle->to_string()); }, @""); } - (NSString *)description { @@ -155,12 +149,12 @@ @implementation ODRTextStyle + (instancetype)styleWithHandle:(const odr::TextStyle &)handle { ODRTextStyle *const result = [[ODRTextStyle alloc] init]; - result->_fontName = box_string_view(handle.font_name); + result->_fontName = box_string(handle.font_name); result->_fontSize = box(handle.font_size); result->_fontWeight = box_enum(handle.font_weight); result->_fontStyle = box_enum(handle.font_style); - result->_fontUnderline = box_bool(handle.font_underline); - result->_fontLineThrough = box_bool(handle.font_line_through); + result->_fontUnderline = box_number(handle.font_underline); + result->_fontLineThrough = box_number(handle.font_line_through); result->_fontShadow = box_string(handle.font_shadow); result->_fontColor = box(handle.font_color); result->_backgroundColor = box(handle.background_color); @@ -223,7 +217,7 @@ + (instancetype)styleWithHandle:(const odr::TableCellStyle &)handle { result->_padding = [ODRDirectionalMeasure directionalWithHandle:handle.padding]; result->_border = [ODRDirectionalString directionalWithHandle:handle.border]; - result->_textRotation = box_double(handle.text_rotation); + result->_textRotation = box_number(handle.text_rotation); return result; } diff --git a/apple/src/ODRTable.mm b/apple/src/ODRTable.mm index eeeb5313d..e43d21065 100644 --- a/apple/src/ODRTable.mm +++ b/apple/src/ODRTable.mm @@ -6,6 +6,7 @@ #include using odr::apple::guarded; +using odr::apple::guarded_value; using odr::apple::to_nsstring; using odr::apple::to_string; @@ -16,25 +17,37 @@ @implementation ODRTableAddress +// The parses throw on anything that is not a cell address — an empty string, a +// lowercase column, a non-numeric row. These have no error out-parameter, so +// they fall back to 0; `position:fromString:` is the one that reports why. + (uint32_t)columnNumberFromString:(NSString *)string { - return odr::TablePosition::to_column_num(to_string(string)); + return guarded_value( + [&] { return odr::TablePosition::to_column_num(to_string(string)); }, 0u); } + (uint32_t)rowNumberFromString:(NSString *)string { - return odr::TablePosition::to_row_num(to_string(string)); + return guarded_value( + [&] { return odr::TablePosition::to_row_num(to_string(string)); }, 0u); } + (NSString *)stringFromColumnNumber:(uint32_t)column { - return to_nsstring(odr::TablePosition::to_column_string(column)); + return guarded_value( + [&] { return to_nsstring(odr::TablePosition::to_column_string(column)); }, + @""); } + (NSString *)stringFromRowNumber:(uint32_t)row { - return to_nsstring(odr::TablePosition::to_row_string(row)); + return guarded_value( + [&] { return to_nsstring(odr::TablePosition::to_row_string(row)); }, @""); } + (NSString *)stringFromPosition:(ODRTablePosition)position { - return to_nsstring( - odr::TablePosition(position.column, position.row).to_string()); + return guarded_value( + [&] { + return to_nsstring( + odr::TablePosition(position.column, position.row).to_string()); + }, + @""); } + (BOOL)position:(ODRTablePosition *)position diff --git a/apple/swift/Element+Tree.swift b/apple/swift/Element+Tree.swift index ccb066228..2aa641b13 100644 --- a/apple/swift/Element+Tree.swift +++ b/apple/swift/Element+Tree.swift @@ -23,7 +23,7 @@ extension Element { /// The first descendant of the given type, or `nil`. public func firstDescendant(ofType type: T.Type) -> T? { - descendants.lazy.compactMap { $0 as? T }.first { _ in true } + descendants(ofType: type).first { _ in true } } /// The chain of ancestors, closest first. diff --git a/apple/swift/HttpServer+Serve.swift b/apple/swift/HttpServer+Serve.swift index 3fb5fd959..b8f39c68a 100644 --- a/apple/swift/HttpServer+Serve.swift +++ b/apple/swift/HttpServer+Serve.swift @@ -1,17 +1,14 @@ import Foundation extension HttpServer { - /// Binds, serves, and stops when the returned handle is released or - /// cancelled. + /// Binds and serves, stopping when the returned handle is released. /// - /// `listen()` blocks its thread until `stop()`, which is a shape no Swift - /// caller wants to manage by hand — and getting it wrong deadlocks, because - /// `stop()` waits for `listen()` to return and so must never be called from - /// the thread that is inside it. This runs `listen()` on a detached thread of - /// its own and hands back the port. Returns only once the server is actually - /// serving, and throws rather than hand back a handle if it never gets there. + /// Runs the blocking `listen()` on a thread of its own — `stop()` waits for + /// `listen()` to return, so calling it from that thread deadlocks. Returns + /// only once the server is really serving, and throws instead of handing back + /// a handle if it never gets there. /// - /// Bind `127.0.0.1` on iOS. `0.0.0.0` trips the Local Network permission + /// Bind `127.0.0.1` on iOS: `0.0.0.0` trips the Local Network permission /// prompt, and nothing off the device needs to reach a server that exists to /// feed a web view. public func serve( @@ -32,12 +29,8 @@ extension HttpServer { thread.name = "app.opendocument.OdrCore.HttpServer" thread.start() - // `listen()` runs on that thread, so `serve()` would otherwise return - // before the server is actually serving and `isRunning` would be false to - // the caller that just started it. Connections queue in the backlog from - // `bind()` onward, so this is about the observable state being honest - // rather than about correctness of the first request. Bounded, because a - // server that was already stopped never starts running at all. + // Otherwise `isRunning` would be false to the caller that just started the + // server. Bounded, because one that was already stopped never starts. let deadline = Date().addingTimeInterval(5) while !isRunning, failure.stored == nil, Date() < deadline { Thread.sleep(forTimeInterval: 0.005) diff --git a/apple/swift/OdrCore.swift b/apple/swift/OdrCore.swift index dcbedeec9..e888527bb 100644 --- a/apple/swift/OdrCore.swift +++ b/apple/swift/OdrCore.swift @@ -1,11 +1,9 @@ /// The Objective-C bindings are the API; this target re-exports them so a /// consumer writes one `import OdrCore`. /// -/// What lives here is only what an ObjC annotation cannot express — sequences -/// over the element tree, real Swift optionals over the boxed style values, and -/// structured concurrency around the blocking HTTP server. Anything that *can* -/// be said with `NS_SWIFT_NAME`, nullability or `NS_ERROR_ENUM` belongs in the -/// headers instead, so there is one API to keep correct rather than two. This -/// is the same rule `android/` follows in refusing to restate the java API in -/// kotlin. +/// Only what an ObjC annotation cannot express belongs here — element-tree +/// sequences, real Swift optionals over the boxed style values, a thread around +/// the blocking HTTP server. Anything `NS_SWIFT_NAME`, nullability or +/// `NS_ERROR_ENUM` can say belongs in the headers, so there is one API to keep +/// correct rather than two. @_exported import OdrCoreObjC diff --git a/apple/tests/Fixture.swift b/apple/tests/Fixture.swift index 29c7d7dfb..68def1a62 100644 --- a/apple/tests/Fixture.swift +++ b/apple/tests/Fixture.swift @@ -1,19 +1,13 @@ import Foundation import XCTest -/// The document the suite runs on. -/// -/// `Fixtures/mixed-layout.odt` is `odt/mixed-layout.odt` from +/// The document the suite runs on: `odt/mixed-layout.odt` from /// [OpenDocument.test](https://github.com/opendocument-app/OpenDocument.test), -/// copied in rather than referenced. `test/data/` is fetched by -/// `cmake/test_data.cmake` and is not part of a package checkout, and pulling -/// it in as a submodule is precisely what `Package.swift` must not do — SwiftPM -/// initialises submodules on every consumer's checkout. +/// copied in because `test/data/` is fetched and a package checkout has none of +/// it — and a submodule is the one thing `Package.swift` must never grow. /// -/// 9 KB of real LibreOffice output, four paragraphs across three master pages, -/// each a text run plus a span. It replaced a document this suite wrote itself, -/// which only ever proved that odrcore could read back what the test had -/// written. +/// 9 KB of real LibreOffice output: four paragraphs across three master pages, +/// each a text run plus a span. enum Fixture { /// The text nodes of `odt`, in document order. Each paragraph is a run and a /// span, so the numbers are their own nodes. diff --git a/cli/src/back_translate.cpp b/cli/src/back_translate.cpp index 22e186ad3..656ada4d3 100644 --- a/cli/src/back_translate.cpp +++ b/cli/src/back_translate.cpp @@ -9,7 +9,12 @@ using namespace odr; -int main(int, char **argv) { +int main(const int argc, char **argv) { + if (argc < 4) { + std::cerr << "usage: back_translate \n"; + return 2; + } + try { const Logger logger = Logger::create_stdio("odr-back-translate", LogLevel::verbose); diff --git a/cli/src/meta.cpp b/cli/src/meta.cpp index dacd903c8..579c85fd1 100644 --- a/cli/src/meta.cpp +++ b/cli/src/meta.cpp @@ -10,6 +10,11 @@ using namespace odr; int main(const int argc, char **argv) { + if (argc < 2) { + std::cerr << "usage: meta [password]\n"; + return 2; + } + try { const Logger logger = Logger::create_stdio("odr-meta", LogLevel::verbose); diff --git a/cli/src/server.cpp b/cli/src/server.cpp index 263fc9791..034765cf5 100644 --- a/cli/src/server.cpp +++ b/cli/src/server.cpp @@ -13,6 +13,11 @@ using namespace odr; int main(const int argc, char **argv) { + if (argc < 2) { + std::cerr << "usage: server [password]\n"; + return 2; + } + try { const Logger logger = Logger::create_stdio("odr-server", LogLevel::verbose); diff --git a/cli/src/translate.cpp b/cli/src/translate.cpp index 212c03e5d..fbb28b7d3 100644 --- a/cli/src/translate.cpp +++ b/cli/src/translate.cpp @@ -9,6 +9,11 @@ using namespace odr; int main(const int argc, char **argv) { + if (argc < 3) { + std::cerr << "usage: translate [password]\n"; + return 2; + } + try { const Logger logger = Logger::create_stdio("odr-translate", LogLevel::verbose); @@ -40,11 +45,9 @@ int main(const int argc, char **argv) { config.editable = true; config.format_html = true; - const std::string output_tmp = output + "/tmp"; - std::filesystem::create_directories(output_tmp); + std::filesystem::create_directories(output); const HtmlService service = html::translate(decoded_file, output, config); - Html html = service.bring_offline(output); - std::filesystem::remove_all(output_tmp); + const Html html = service.bring_offline(output); return 0; } catch (const std::exception &e) { diff --git a/jni/java/app/opendocument/core/GuardedNativeResource.java b/jni/java/app/opendocument/core/GuardedNativeResource.java index 4ba147112..80032977a 100644 --- a/jni/java/app/opendocument/core/GuardedNativeResource.java +++ b/jni/java/app/opendocument/core/GuardedNativeResource.java @@ -6,14 +6,9 @@ * A {@link NativeResource} whose handle stays valid for the duration of a native * call, even when another thread closes it meanwhile. * - *

{@link NativeResource#handle()} hands out a raw pointer and {@link #close()} - * frees what it points at, so a call that has read the handle but not used it yet - * is left holding a dangling one. For nearly every binding that is caller error - - * you do not use an object while you close it - and plain {@link NativeResource} - * is right, at no cost in fields or locks. It is not caller error where the API - * asks for the call to be made on a thread of its own, which is - * {@code HttpServer.listen()}: closing the server is exactly how that call is - * meant to end. + *

Using an object while closing it is caller error everywhere except where the + * API asks for the call to run on a thread of its own - {@code HttpServer.listen()}, + * which closing the server is meant to end. * *

{@link #guarded} counts such a call in and out. {@link #close()} stops new * ones from starting, calls {@link #unblock()} to make the ones in flight return, diff --git a/jni/java/app/opendocument/core/Html.java b/jni/java/app/opendocument/core/Html.java index 983bcef4f..b3e021bee 100644 --- a/jni/java/app/opendocument/core/Html.java +++ b/jni/java/app/opendocument/core/Html.java @@ -51,19 +51,35 @@ public static final class LocatedResource { } } + // The handles below go into a static native as arguments, so no receiver holds + // the wrapper for the duration - keepAlive() does. + /** Translates a decoded file to HTML. */ public static HtmlService translate(DecodedFile file, String cachePath, HtmlConfig config) { - return new HtmlService(translateFile(file.handle(), cachePath, config), file); + try { + return new HtmlService(translateFile(file.handle(), cachePath, config), file); + } finally { + file.keepAlive(); + } } /** Translates a document to HTML. */ public static HtmlService translate(Document document, String cachePath, HtmlConfig config) { - return new HtmlService(translateDocument(document.handle(), cachePath, config), document); + try { + return new HtmlService(translateDocument(document.handle(), cachePath, config), document); + } finally { + document.keepAlive(); + } } /** Translates a filesystem to HTML. */ public static HtmlService translate(Filesystem filesystem, String cachePath, HtmlConfig config) { - return new HtmlService(translateFilesystem(filesystem.handle(), cachePath, config), filesystem); + try { + return new HtmlService( + translateFilesystem(filesystem.handle(), cachePath, config), filesystem); + } finally { + filesystem.keepAlive(); + } } /** Applies a diff (produced by the browser-side editor) to a document. */ diff --git a/jni/java/app/opendocument/core/HtmlService.java b/jni/java/app/opendocument/core/HtmlService.java index 3b71b00a2..d697b0124 100644 --- a/jni/java/app/opendocument/core/HtmlService.java +++ b/jni/java/app/opendocument/core/HtmlService.java @@ -59,7 +59,14 @@ public Html bringOffline(String outputPath, List views) { for (int i = 0; i < handles.length; i++) { handles[i] = views.get(i).handle(); } - return bringOfflineViewsNative(handle(), outputPath, handles); + try { + return bringOfflineViewsNative(handle(), outputPath, handles); + } finally { + // the handles went in as arguments, so nothing else keeps the views alive + for (HtmlView view : views) { + view.keepAlive(); + } + } } private static native void destroy(long handle); diff --git a/jni/java/app/opendocument/core/HttpServer.java b/jni/java/app/opendocument/core/HttpServer.java index 7be89e670..ac339e34b 100644 --- a/jni/java/app/opendocument/core/HttpServer.java +++ b/jni/java/app/opendocument/core/HttpServer.java @@ -65,9 +65,8 @@ public int bind(String host, int port, Options options) { * Blocks serving requests until {@link #stop()} is called from another thread. * Returns right away if the server has already been stopped or closed. * - *

Guarded: this is the one call in the bindings that is meant to be made on a - * thread of its own while another closes the object it runs on, so the handle it - * takes has to stay valid until it hands it back. + *

Guarded: the handle stays valid until this returns, even if another thread + * closes the server meanwhile. */ public void listen() { guarded(this::listenNative); diff --git a/jni/java/app/opendocument/core/NativeResource.java b/jni/java/app/opendocument/core/NativeResource.java index c5ebc9ced..522e310fa 100644 --- a/jni/java/app/opendocument/core/NativeResource.java +++ b/jni/java/app/opendocument/core/NativeResource.java @@ -17,11 +17,8 @@ * collected while handles into it are alive. * *

The post-mortem free is a {@link PhantomReference} drained by a daemon - * thread rather than a {@link java.lang.ref.Cleaner}, which is exactly what a - * Cleaner does internally. Cleaner is a JDK 9 API that android only ships from - * API level 33, and OpenDocument.droid targets API 26; referencing it made the - * whole class fail to load there with a {@code NoClassDefFoundError}, and core - * library desugaring does not cover {@code java.lang.ref}. + * thread, not a {@link java.lang.ref.Cleaner}: android ships Cleaner only from + * API 33 and desugaring does not cover {@code java.lang.ref}. */ public abstract class NativeResource implements AutoCloseable { private static final ReferenceQueue QUEUE = new ReferenceQueue<>(); @@ -82,24 +79,12 @@ final boolean isClosed() { } /** - * A use of this object that the optimiser cannot drop, so it stays reachable - * across whatever ran before it. + * A use the optimiser cannot drop, keeping this object reachable across the + * call before it. Needed for handles passed as arguments, where no receiver + * holds the wrapper and the reaper could free the handle mid-call. * - *

{@link #handle()} outlives the wrapper it came from: once nothing refers to - * the wrapper any more the collector may enqueue it and the reaper may free the - * handle, and the just-in-time compiler is free to decide that while a native - * call using it is still running. A native that takes the handle of the object it - * is called on is safe without this - the JNI frame holds the receiver - which is - * why every one of them is an instance method of its owner. This is for the - * handles that go in as arguments, where there is no receiver to hold them. - * - *

{@code java.lang.ref.Reference.reachabilityFence} is the API for it, and is - * unusable here: android has it from API level 28, {@code android/build.gradle.kts} - * sets {@code minSdk = 26}, and core library desugaring does not cover - * {@code java.lang.ref} - the same wall {@link java.lang.ref.Cleaner} hit above. - * Taking the monitor of an object the reference queue can also see is the fallback - * the JDK itself used before that method existed: lock elision needs the object to - * be provably confined, and this one is not. + *

The monitor stands in for {@code Reference.reachabilityFence}, which + * android only has from API 28 (see {@code jni/AGENTS.md}). */ final void keepAlive() { synchronized (this) { diff --git a/jni/src/jni_core.cpp b/jni/src/jni_core.cpp index c5d1e2843..73daf0de6 100644 --- a/jni/src/jni_core.cpp +++ b/jni/src/jni_core.cpp @@ -254,17 +254,15 @@ Java_app_opendocument_core_Odr_openWithPreferenceNative( if (as_file_type >= 0) { preference.as_file_type = static_cast(as_file_type); } - const auto append_codes = [&](jintArray array, auto &target, - auto transform) { - const jsize length = env->GetArrayLength(array); - jint *codes = env->GetIntArrayElements(array, nullptr); + if (jint *codes = env->GetIntArrayElements(file_type_priority, nullptr); + codes != nullptr) { + const jsize length = env->GetArrayLength(file_type_priority); for (jsize i = 0; i < length; ++i) { - target.push_back(transform(codes[i])); + preference.file_type_priority.push_back( + static_cast(codes[i])); } - env->ReleaseIntArrayElements(array, codes, JNI_ABORT); - }; - append_codes(file_type_priority, preference.file_type_priority, - [](jint code) { return static_cast(code); }); + env->ReleaseIntArrayElements(file_type_priority, codes, JNI_ABORT); + } return make_handle(odr::open(to_string(env, path), preference)); }); } diff --git a/jni/src/jni_document.cpp b/jni/src/jni_document.cpp index 8add6671f..487cdc10f 100644 --- a/jni/src/jni_document.cpp +++ b/jni/src/jni_document.cpp @@ -50,13 +50,12 @@ jlongArray wrap_elements(JNIEnv *env, const odr::ElementRange &range) { // app.opendocument.core.Document extern "C" JNIEXPORT void JNICALL -Java_app_opendocument_core_Document_destroy(JNIEnv *, jclass, jlong handle) { - destroy_handle(handle); +Java_app_opendocument_core_Document_destroy(JNIEnv *env, jclass, jlong handle) { + destroy_handle(env, handle); } -// odr::html::edit, but it belongs to Document: a native that takes a handle has -// to be an instance method of whatever owns it, or the wrapper can be collected -// - and the handle freed - while the call is still running +// odr::html::edit, but it belongs to Document: a native taking a handle must be +// an instance method of its owner, or the wrapper can be collected mid-call. extern "C" JNIEXPORT void JNICALL Java_app_opendocument_core_Document_editNative(JNIEnv *env, jobject, jlong handle, jstring diff) { @@ -147,9 +146,9 @@ Java_app_opendocument_core_DocumentPath_create(JNIEnv *env, jclass, } extern "C" JNIEXPORT void JNICALL -Java_app_opendocument_core_DocumentPath_destroy(JNIEnv *, jclass, +Java_app_opendocument_core_DocumentPath_destroy(JNIEnv *env, jclass, jlong handle) { - destroy_handle(handle); + destroy_handle(env, handle); } extern "C" JNIEXPORT jboolean JNICALL @@ -190,8 +189,8 @@ Java_app_opendocument_core_DocumentPath_toStringNative(JNIEnv *env, jobject, // app.opendocument.core.Element extern "C" JNIEXPORT void JNICALL -Java_app_opendocument_core_Element_destroy(JNIEnv *, jclass, jlong handle) { - destroy_handle(handle); +Java_app_opendocument_core_Element_destroy(JNIEnv *env, jclass, jlong handle) { + destroy_handle(env, handle); } extern "C" JNIEXPORT jint JNICALL Java_app_opendocument_core_Element_typeNative( diff --git a/jni/src/jni_file.cpp b/jni/src/jni_file.cpp index 31d9c6388..9331d8f3c 100644 --- a/jni/src/jni_file.cpp +++ b/jni/src/jni_file.cpp @@ -33,8 +33,8 @@ Java_app_opendocument_core_File_create(JNIEnv *env, jclass, jstring path) { } extern "C" JNIEXPORT void JNICALL -Java_app_opendocument_core_File_destroy(JNIEnv *, jclass, jlong handle) { - destroy_handle(handle); +Java_app_opendocument_core_File_destroy(JNIEnv *env, jclass, jlong handle) { + destroy_handle(env, handle); } extern "C" JNIEXPORT jint JNICALL @@ -76,9 +76,8 @@ extern "C" JNIEXPORT void JNICALL Java_app_opendocument_core_File_copyNative( [&] { from_handle(handle)->copy(to_string(env, path)); }); } -// yields a DecodedFile but belongs to File: a native that takes a handle has to -// be an instance method of whatever owns it, or the wrapper can be collected - -// and the handle freed - while the call is still running +// Yields a DecodedFile but belongs to File: a native taking a handle must be an +// instance method of its owner, or the wrapper can be collected mid-call. extern "C" JNIEXPORT jlong JNICALL Java_app_opendocument_core_File_decodeNative( JNIEnv *env, jobject, jlong handle) { return guarded(env, [&] { @@ -108,8 +107,9 @@ Java_app_opendocument_core_DecodedFile_createAs(JNIEnv *env, jclass, } extern "C" JNIEXPORT void JNICALL -Java_app_opendocument_core_DecodedFile_destroy(JNIEnv *, jclass, jlong handle) { - destroy_handle(handle); +Java_app_opendocument_core_DecodedFile_destroy(JNIEnv *env, jclass, + jlong handle) { + destroy_handle(env, handle); } extern "C" JNIEXPORT jlong JNICALL @@ -403,9 +403,9 @@ Java_app_opendocument_core_FontFile_readNative(JNIEnv *env, jobject, // app.opendocument.core.FileWalker -extern "C" JNIEXPORT void JNICALL -Java_app_opendocument_core_FileWalker_destroy(JNIEnv *, jclass, jlong handle) { - destroy_handle(handle); +extern "C" JNIEXPORT void JNICALL Java_app_opendocument_core_FileWalker_destroy( + JNIEnv *env, jclass, jlong handle) { + destroy_handle(env, handle); } extern "C" JNIEXPORT jboolean JNICALL @@ -470,9 +470,9 @@ Java_app_opendocument_core_FileWalker_flatNextNative(JNIEnv *env, jobject, // app.opendocument.core.Filesystem -extern "C" JNIEXPORT void JNICALL -Java_app_opendocument_core_Filesystem_destroy(JNIEnv *, jclass, jlong handle) { - destroy_handle(handle); +extern "C" JNIEXPORT void JNICALL Java_app_opendocument_core_Filesystem_destroy( + JNIEnv *env, jclass, jlong handle) { + destroy_handle(env, handle); } extern "C" JNIEXPORT jboolean JNICALL @@ -526,8 +526,8 @@ Java_app_opendocument_core_Filesystem_openNative(JNIEnv *env, jobject, // app.opendocument.core.Archive extern "C" JNIEXPORT void JNICALL -Java_app_opendocument_core_Archive_destroy(JNIEnv *, jclass, jlong handle) { - destroy_handle(handle); +Java_app_opendocument_core_Archive_destroy(JNIEnv *env, jclass, jlong handle) { + destroy_handle(env, handle); } extern "C" JNIEXPORT jlong JNICALL diff --git a/jni/src/jni_html.cpp b/jni/src/jni_html.cpp index 6c070a199..a9fa42f8e 100644 --- a/jni/src/jni_html.cpp +++ b/jni/src/jni_html.cpp @@ -53,11 +53,13 @@ jobject make_html(JNIEnv *env, odr::Html html) { jobjectArray page_array = env->NewObjectArray(static_cast(pages.size()), page_cls, nullptr); for (jsize i = 0; i < static_cast(pages.size()); ++i) { - jobject page = - env->NewObject(page_cls, page_ctor, to_jstring(env, pages[i].name), - to_jstring(env, pages[i].path)); + jstring name = to_jstring(env, pages[i].name); + jstring path = to_jstring(env, pages[i].path); + jobject page = env->NewObject(page_cls, page_ctor, name, path); env->SetObjectArrayElement(page_array, i, page); env->DeleteLocalRef(page); + env->DeleteLocalRef(path); + env->DeleteLocalRef(name); } env->DeleteLocalRef(page_cls); @@ -94,11 +96,15 @@ jobject make_content(JNIEnv *env, const std::string &html, const auto &[resource, location] = resources[i]; jobject resource_obj = env->NewObject( resource_cls, resource_ctor, make_handle(odr::HtmlResource(resource))); - jobject located = env->NewObject( - located_cls, located_ctor, resource_obj, - location.has_value() ? to_jstring(env, *location) : nullptr); + jstring location_str = + location.has_value() ? to_jstring(env, *location) : nullptr; + jobject located = + env->NewObject(located_cls, located_ctor, resource_obj, location_str); env->SetObjectArrayElement(located_array, i, located); env->DeleteLocalRef(located); + if (location_str != nullptr) { + env->DeleteLocalRef(location_str); + } env->DeleteLocalRef(resource_obj); } env->DeleteLocalRef(resource_cls); @@ -174,8 +180,9 @@ Java_app_opendocument_core_Html_translateFilesystem(JNIEnv *env, jclass, // app.opendocument.core.HtmlService extern "C" JNIEXPORT void JNICALL -Java_app_opendocument_core_HtmlService_destroy(JNIEnv *, jclass, jlong handle) { - destroy_handle(handle); +Java_app_opendocument_core_HtmlService_destroy(JNIEnv *env, jclass, + jlong handle) { + destroy_handle(env, handle); } extern "C" JNIEXPORT jobject JNICALL @@ -269,8 +276,8 @@ Java_app_opendocument_core_HtmlService_bringOfflineViewsNative( // app.opendocument.core.HtmlView extern "C" JNIEXPORT void JNICALL -Java_app_opendocument_core_HtmlView_destroy(JNIEnv *, jclass, jlong handle) { - destroy_handle(handle); +Java_app_opendocument_core_HtmlView_destroy(JNIEnv *env, jclass, jlong handle) { + destroy_handle(env, handle); } extern "C" JNIEXPORT jstring JNICALL @@ -322,9 +329,9 @@ Java_app_opendocument_core_HtmlView_bringOfflineNative(JNIEnv *env, jobject, // app.opendocument.core.HtmlResource extern "C" JNIEXPORT void JNICALL -Java_app_opendocument_core_HtmlResource_destroy(JNIEnv *, jclass, +Java_app_opendocument_core_HtmlResource_destroy(JNIEnv *env, jclass, jlong handle) { - destroy_handle(handle); + destroy_handle(env, handle); } extern "C" JNIEXPORT jint JNICALL diff --git a/jni/src/jni_http_server.cpp b/jni/src/jni_http_server.cpp index ea4a2913e..1a20b52e4 100644 --- a/jni/src/jni_http_server.cpp +++ b/jni/src/jni_http_server.cpp @@ -32,9 +32,9 @@ Java_app_opendocument_core_HttpServer_create(JNIEnv *env, jclass) { return guarded(env, [&] { return make_handle(odr::HttpServer()); }); } -extern "C" JNIEXPORT void JNICALL -Java_app_opendocument_core_HttpServer_destroy(JNIEnv *, jclass, jlong handle) { - destroy_handle(handle); +extern "C" JNIEXPORT void JNICALL Java_app_opendocument_core_HttpServer_destroy( + JNIEnv *env, jclass, jlong handle) { + destroy_handle(env, handle); } extern "C" JNIEXPORT void JNICALL diff --git a/jni/src/jni_logger.cpp b/jni/src/jni_logger.cpp index 2571f4b97..438af5fac 100644 --- a/jni/src/jni_logger.cpp +++ b/jni/src/jni_logger.cpp @@ -17,9 +17,7 @@ using odr_jni::to_jstring; using odr_jni::to_string; /// `JavaVM::AttachCurrentThread` takes a `JNIEnv **` on android and a `void **` -/// on the jdk, and neither pointer converts to the other, so the argument has -/// to be typed per platform. `GetEnv` is `void **` on both and needs none of -/// this. +/// on the jdk, and neither converts to the other. `GetEnv` needs none of this. jint attach_current_thread(JavaVM *const vm, JNIEnv **const env) { #ifdef __ANDROID__ return vm->AttachCurrentThread(env, nullptr); @@ -81,6 +79,7 @@ class JavaLogger final : public odr::ILogger { return; } m_bridge = static_cast(env->NewGlobalRef(bridge)); + env->DeleteLocalRef(bridge); m_will_log = env->GetStaticMethodID(m_bridge, "willLog", "(Lapp/opendocument/core/ILogger;I)Z"); m_log = env->GetStaticMethodID(m_bridge, "log", @@ -135,6 +134,11 @@ class JavaLogger final : public odr::ILogger { static_cast(level), text, file_name, function_name, static_cast(location.line())); clear_pending(env.get()); + // an already-attached thread keeps one frame for the whole native call, so + // the strings of every message logged during it would pile up in it + env.get()->DeleteLocalRef(text); + env.get()->DeleteLocalRef(function_name); + env.get()->DeleteLocalRef(file_name); } void flush() override { @@ -220,6 +224,6 @@ extern "C" JNIEXPORT void JNICALL Java_app_opendocument_core_Logger_flushNative( } extern "C" JNIEXPORT void JNICALL -Java_app_opendocument_core_Logger_destroy(JNIEnv *, jclass, jlong handle) { - destroy_handle(handle); +Java_app_opendocument_core_Logger_destroy(JNIEnv *env, jclass, jlong handle) { + destroy_handle(env, handle); } diff --git a/jni/src/jni_style.cpp b/jni/src/jni_style.cpp index c7695207d..9208ba996 100644 --- a/jni/src/jni_style.cpp +++ b/jni/src/jni_style.cpp @@ -97,9 +97,16 @@ jobject enum_from_code(JNIEnv *env, const char *class_name, const jint code) { } auto array = static_cast(env->CallStaticObjectMethod(cls, values)); - jobject result = env->GetObjectArrayElement(array, code); - env->DeleteLocalRef(array); env->DeleteLocalRef(cls); + if (array == nullptr) { + return nullptr; + } + // out of range means the java enum lags the C++ one; every JNI call after a + // pending ArrayIndexOutOfBoundsException would be undefined + jobject result = code < env->GetArrayLength(array) + ? env->GetObjectArrayElement(array, code) + : nullptr; + env->DeleteLocalRef(array); return result; } @@ -142,12 +149,7 @@ jobject make_measure(JNIEnv *env, const odr::Measure &value) { } jobject make_measure(JNIEnv *env, const std::optional &value) { - if (!value.has_value()) { - return nullptr; - } - return new_object(env, "app/opendocument/core/Measure", - "(DLjava/lang/String;)V", value->magnitude(), - to_jstring(env, value->unit().to_string())); + return value.has_value() ? make_measure(env, *value) : nullptr; } jobject make_color(JNIEnv *env, const std::optional &value) { @@ -380,8 +382,9 @@ jobject html_config_to_java(JNIEnv *env, const odr::HtmlConfig &config) { for (jsize i = 0; i < static_cast(config.pdf_dual_layer_fallback_fonts.size()); ++i) { - env->SetObjectArrayElement( - fonts, i, to_jstring(env, config.pdf_dual_layer_fallback_fonts[i])); + jstring font = to_jstring(env, config.pdf_dual_layer_fallback_fonts[i]); + env->SetObjectArrayElement(fonts, i, font); + env->DeleteLocalRef(font); } set_object("pdfDualLayerFallbackFonts", "[Ljava/lang/String;", fonts); env->DeleteLocalRef(string_cls); diff --git a/jni/src/odr_jni.hpp b/jni/src/odr_jni.hpp index f7e78717e..0427d3a8c 100644 --- a/jni/src/odr_jni.hpp +++ b/jni/src/odr_jni.hpp @@ -44,8 +44,10 @@ template jlong make_handle(T value) { return reinterpret_cast(new T(std::move(value))); } -template void destroy_handle(jlong handle) { - delete from_handle(handle); +/// Frees a handle. Guarded like every other native body: a destructor that +/// throws would otherwise unwind through the JNI boundary. +template void destroy_handle(JNIEnv *env, jlong handle) { + guarded(env, [&] { delete from_handle(handle); }); } } // namespace odr_jni diff --git a/python/pyodr/cli.py b/python/pyodr/cli.py index b4c0008cf..6428f8ec8 100644 --- a/python/pyodr/cli.py +++ b/python/pyodr/cli.py @@ -60,17 +60,20 @@ def _serve(args, file) -> int: print("pyodr was built without the HTTP server", file=sys.stderr) return 1 - server_config = pyodr.HttpServer.Config() - server_config.cache_path = tempfile.mkdtemp(prefix="pyodr-server-") - server = pyodr.HttpServer(server_config) - html_config = pyodr.HtmlConfig() html_config.embed_images = False + cache = tempfile.mkdtemp(prefix="pyodr-server-") + service = pyodr.html.translate(file, cache, html_config) prefix = "file" - views = server.serve_file(file, prefix, html_config) + server = pyodr.HttpServer() + server.connect_service(service, prefix) + # bind before printing: the port is only known once the socket is, and it is + # not necessarily the one that was asked for + port = server.bind(args.host, args.port) urls = [ - f"http://{args.host}:{args.port}/file/{prefix}/{view.path()}" for view in views + f"http://{args.host}:{port}/file/{prefix}/{view.path()}" + for view in service.list_views() ] for url in urls: print(url) @@ -80,7 +83,7 @@ def _serve(args, file) -> int: # `listen` blocks in C++; restore the default SIGINT handler so Ctrl+C # terminates the server. signal.signal(signal.SIGINT, signal.SIG_DFL) - server.listen(args.host, args.port) + server.listen() return 0 diff --git a/python/src/bind_core.cpp b/python/src/bind_core.cpp index b11063e0d..56718059b 100644 --- a/python/src/bind_core.cpp +++ b/python/src/bind_core.cpp @@ -69,12 +69,8 @@ void odr_python::bind_core(py::module_ &m) { void odr_python::bind_functions(py::module_ &m) { m.def("all_file_types", &odr::all_file_types, "Every file type this library knows about."); - m.def( - "file_type_by_file_extension", - [](const std::string &extension) { - return odr::file_type_by_file_extension(extension); - }, - py::arg("extension")); + m.def("file_type_by_file_extension", &odr::file_type_by_file_extension, + py::arg("extension")); m.def( "file_extensions_by_file_type", [](const odr::FileType type) { diff --git a/python/src/bind_document.cpp b/python/src/bind_document.cpp index 481b44b87..3e4fbbf65 100644 --- a/python/src/bind_document.cpp +++ b/python/src/bind_document.cpp @@ -17,18 +17,29 @@ namespace py = pybind11; namespace { -// Ties the returned object to `self` so navigation handles keep the -// originating `Document` alive transitively. +// Ties the returned object to `self`: navigation handles keep the originating +// `Document` alive transitively, and so does a `TextStyle`, whose `font_name` +// borrows from the document. constexpr auto keep_self_alive = py::keep_alive<0, 1>(); py::object make_element_iterator(const odr::ElementRange &range) { - return py::make_iterator(range.begin(), range.end()); + // The elements need the same tie: pybind11 hands them out by value, which + // drops the keep-alive `reference_internal` would otherwise imply. + return py::make_iterator(range.begin(), range.end(), keep_self_alive); } py::object make_children_iterator(const odr::Element &element) { return make_element_iterator(element.children()); } +/// `__bool__` has to come from the derived type: `Element::operator bool` +/// ignores the typed adapter, so a failed `as_*` cast would look valid. +template +py::class_ bind_element(py::module_ &m, const char *name) { + return py::class_(m, name).def("__bool__", + &T::operator bool); +} + } // namespace void odr_python::bind_document(py::module_ &m) { @@ -162,17 +173,17 @@ void odr_python::bind_document(py::module_ &m) { .def("as_custom_shape", &odr::Element::as_custom_shape, keep_self_alive) .def("as_image", &odr::Element::as_image, keep_self_alive); - py::class_(m, "TextRoot") + bind_element(m, "TextRoot") .def("page_layout", &odr::TextRoot::page_layout) .def("first_master_page", &odr::TextRoot::first_master_page, keep_self_alive); - py::class_(m, "Slide") + bind_element(m, "Slide") .def("name", &odr::Slide::name) .def("page_layout", &odr::Slide::page_layout) .def("master_page", &odr::Slide::master_page, keep_self_alive); - py::class_(m, "Sheet") + bind_element(m, "Sheet") .def("name", &odr::Sheet::name) .def("dimensions", &odr::Sheet::dimensions) .def("content", &odr::Sheet::content, py::arg("range")) @@ -190,44 +201,43 @@ void odr_python::bind_document(py::module_ &m) { .def("cell_style", &odr::Sheet::cell_style, py::arg("column"), py::arg("row")); - py::class_(m, "SheetCell") + bind_element(m, "SheetCell") .def("position", &odr::SheetCell::position) .def("is_covered", &odr::SheetCell::is_covered) .def("span", &odr::SheetCell::span) .def("value_type", &odr::SheetCell::value_type); - py::class_(m, "Page") + bind_element(m, "Page") .def("name", &odr::Page::name) .def("page_layout", &odr::Page::page_layout) .def("master_page", &odr::Page::master_page, keep_self_alive); - py::class_(m, "MasterPage") + bind_element(m, "MasterPage") .def("page_layout", &odr::MasterPage::page_layout); - py::class_(m, "LineBreak") - .def("style", &odr::LineBreak::style); + bind_element(m, "LineBreak") + .def("style", &odr::LineBreak::style, keep_self_alive); - py::class_(m, "Paragraph") + bind_element(m, "Paragraph") .def("style", &odr::Paragraph::style) - .def("text_style", &odr::Paragraph::text_style); + .def("text_style", &odr::Paragraph::text_style, keep_self_alive); - py::class_(m, "Span").def("style", - &odr::Span::style); + bind_element(m, "Span").def("style", &odr::Span::style, + keep_self_alive); - py::class_(m, "Text") + bind_element(m, "Text") .def("content", &odr::Text::content) .def("set_content", &odr::Text::set_content, py::arg("text")) - .def("style", &odr::Text::style); + .def("style", &odr::Text::style, keep_self_alive); - py::class_(m, "Link").def("href", &odr::Link::href); + bind_element(m, "Link").def("href", &odr::Link::href); - py::class_(m, "Bookmark") - .def("name", &odr::Bookmark::name); + bind_element(m, "Bookmark").def("name", &odr::Bookmark::name); - py::class_(m, "ListItem") - .def("style", &odr::ListItem::style); + bind_element(m, "ListItem") + .def("style", &odr::ListItem::style, keep_self_alive); - py::class_(m, "Table") + bind_element(m, "Table") .def("first_row", &odr::Table::first_row, keep_self_alive) .def("first_column", &odr::Table::first_column, keep_self_alive) .def( @@ -245,19 +255,19 @@ void odr_python::bind_document(py::module_ &m) { .def("dimensions", &odr::Table::dimensions) .def("style", &odr::Table::style); - py::class_(m, "TableColumn") + bind_element(m, "TableColumn") .def("style", &odr::TableColumn::style); - py::class_(m, "TableRow") + bind_element(m, "TableRow") .def("style", &odr::TableRow::style); - py::class_(m, "TableCell") + bind_element(m, "TableCell") .def("is_covered", &odr::TableCell::is_covered) .def("span", &odr::TableCell::span) .def("value_type", &odr::TableCell::value_type) .def("style", &odr::TableCell::style); - py::class_(m, "Frame") + bind_element(m, "Frame") .def("anchor_type", &odr::Frame::anchor_type) .def("x", &odr::Frame::x) .def("y", &odr::Frame::y) @@ -266,35 +276,35 @@ void odr_python::bind_document(py::module_ &m) { .def("z_index", &odr::Frame::z_index) .def("style", &odr::Frame::style); - py::class_(m, "Rect") + bind_element(m, "Rect") .def("x", &odr::Rect::x) .def("y", &odr::Rect::y) .def("width", &odr::Rect::width) .def("height", &odr::Rect::height) .def("style", &odr::Rect::style); - py::class_(m, "Line") + bind_element(m, "Line") .def("x1", &odr::Line::x1) .def("y1", &odr::Line::y1) .def("x2", &odr::Line::x2) .def("y2", &odr::Line::y2) .def("style", &odr::Line::style); - py::class_(m, "Circle") + bind_element(m, "Circle") .def("x", &odr::Circle::x) .def("y", &odr::Circle::y) .def("width", &odr::Circle::width) .def("height", &odr::Circle::height) .def("style", &odr::Circle::style); - py::class_(m, "CustomShape") + bind_element(m, "CustomShape") .def("x", &odr::CustomShape::x) .def("y", &odr::CustomShape::y) .def("width", &odr::CustomShape::width) .def("height", &odr::CustomShape::height) .def("style", &odr::CustomShape::style); - py::class_(m, "Image") + bind_element(m, "Image") .def("is_internal", &odr::Image::is_internal) .def("file", &odr::Image::file) .def("href", &odr::Image::href); diff --git a/python/src/bind_http_server.cpp b/python/src/bind_http_server.cpp index 127155bd5..5650e7f82 100644 --- a/python/src/bind_http_server.cpp +++ b/python/src/bind_http_server.cpp @@ -16,9 +16,8 @@ void odr_python::bind_http_server(py::module_ &m) { py::class_( server, "Config", - "Server-wide settings. Empty since the cache path went with " - "`serve_file`: " - "what a service was translated into belongs to whoever translated it.") + "Server-wide settings, empty for now: what a service was translated " + "into belongs to whoever translated it.") .def(py::init<>()); py::class_(server, "Options", diff --git a/python/tests/test_document.py b/python/tests/test_document.py index 677afb600..f847d513f 100644 --- a/python/tests/test_document.py +++ b/python/tests/test_document.py @@ -59,6 +59,28 @@ def test_element_navigation(odt_path): assert second.previous_sibling() == first +def test_failed_cast_is_falsy(odt_path): + document = pyodr.open(str(odt_path)).as_document_file().document() + paragraph = next( + child + for child in document.root_element() + if child.type() == pyodr.ElementType.paragraph + ) + + assert paragraph.as_paragraph() + # the wrong cast has to come back falsy, not as a valid-looking handle + assert not paragraph.as_slide() + + +def test_children_outlive_the_document(odt_path): + def collect(): + document = pyodr.open(str(odt_path)).as_document_file().document() + return list(document.root_element()) + + # the document is only reachable through the elements by now + assert [child.type() for child in collect()] + + def test_text_root(odt_path): document = pyodr.open(str(odt_path)).as_document_file().document() root = document.root_element().as_text_root() diff --git a/src/odr/document_element.cpp b/src/odr/document_element.cpp index 4cb4d88d0..44149cfb6 100644 --- a/src/odr/document_element.cpp +++ b/src/odr/document_element.cpp @@ -74,105 +74,175 @@ Element Element::navigate_path(const DocumentPath &path) const { } TextRoot Element::as_text_root() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->text_root_adapter(m_identifier)}; } Slide Element::as_slide() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->slide_adapter(m_identifier)}; } Sheet Element::as_sheet() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->sheet_adapter(m_identifier)}; } Page Element::as_page() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->page_adapter(m_identifier)}; } SheetCell Element::as_sheet_cell() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->sheet_cell_adapter(m_identifier)}; } MasterPage Element::as_master_page() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->master_page_adapter(m_identifier)}; } LineBreak Element::as_line_break() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->line_break_adapter(m_identifier)}; } Paragraph Element::as_paragraph() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->paragraph_adapter(m_identifier)}; } Span Element::as_span() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->span_adapter(m_identifier)}; } Text Element::as_text() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->text_adapter(m_identifier)}; } Link Element::as_link() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->link_adapter(m_identifier)}; } Bookmark Element::as_bookmark() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->bookmark_adapter(m_identifier)}; } ListItem Element::as_list_item() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->list_item_adapter(m_identifier)}; } Table Element::as_table() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->table_adapter(m_identifier)}; } TableColumn Element::as_table_column() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->table_column_adapter(m_identifier)}; } TableRow Element::as_table_row() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->table_row_adapter(m_identifier)}; } TableCell Element::as_table_cell() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->table_cell_adapter(m_identifier)}; } Frame Element::as_frame() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->frame_adapter(m_identifier)}; } Rect Element::as_rect() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->rect_adapter(m_identifier)}; } Line Element::as_line() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->line_adapter(m_identifier)}; } Circle Element::as_circle() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->circle_adapter(m_identifier)}; } CustomShape Element::as_custom_shape() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->custom_shape_adapter(m_identifier)}; } Image Element::as_image() const { + if (!exists_()) { + return {}; + } return {m_adapter, m_identifier, m_adapter->image_adapter(m_identifier)}; } ElementRange Element::children() const { - return {exists_() ? ElementIterator(m_adapter, m_adapter->element_first_child( - m_identifier)) - : ElementIterator(), - ElementIterator()}; + if (!exists_()) { + return {}; + } + return ElementRange( + ElementIterator(m_adapter, m_adapter->element_first_child(m_identifier))); } ElementIterator::ElementIterator() = default; diff --git a/src/odr/exceptions.cpp b/src/odr/exceptions.cpp index f4d1a8fe9..1ff5b674d 100644 --- a/src/odr/exceptions.cpp +++ b/src/odr/exceptions.cpp @@ -19,7 +19,7 @@ FileNotFound::FileNotFound(const std::string &path) UnknownFileType::UnknownFileType() : Exception("unknown file type") {} UnsupportedFileType::UnsupportedFileType(const FileType file_type) - : Exception("unknown file type: " + file_type_to_string(file_type)), + : Exception("unsupported file type: " + file_type_to_string(file_type)), file_type{file_type} {} FileReadError::FileReadError() : Exception("file read error") {} diff --git a/src/odr/exceptions.hpp b/src/odr/exceptions.hpp index a5408c706..4e0d2fdf8 100644 --- a/src/odr/exceptions.hpp +++ b/src/odr/exceptions.hpp @@ -6,11 +6,9 @@ namespace odr { enum class FileType; -/// @brief Base of every exception type this library declares. -/// -/// Catching this catches every typed error below. Note that the decoders also -/// throw plain `std::runtime_error` for malformed input that has no dedicated -/// type, so `std::runtime_error` remains the widest net. +/// @brief Base of every exception type this library declares. The decoders also +/// throw plain `std::runtime_error` for malformed input with no dedicated type, +/// so that remains the widest net. struct Exception : std::runtime_error { using std::runtime_error::runtime_error; }; diff --git a/src/odr/file.hpp b/src/odr/file.hpp index 295eebc84..5e3561d59 100644 --- a/src/odr/file.hpp +++ b/src/odr/file.hpp @@ -90,11 +90,9 @@ enum class FileType { // https://en.wikipedia.org/wiki/OpenType opentype_font, - // The media formats a viewer is regularly handed alongside documents. - // Nothing here is decoded - opening one wraps its bytes, and translating it - // puts those bytes in an `` or in a player and lets the browser do the - // work. Naming them is what makes that possible; without a name the text - // fallback would call a video plain text. + // Media a viewer is handed alongside documents. Nothing here is decoded: + // opening one wraps its bytes and translating it hands them to an `` or + // a player - but unnamed, a video would fall back to plain text. // New entries go at the end: the bindings mirror this enum by ordinal. // https://en.wikipedia.org/wiki/WebP webp, @@ -144,9 +142,8 @@ enum class FileType { // https://en.wikipedia.org/wiki/Windows_Metafile#Enhanced_Metafile enhanced_metafile, - // Classification only for now - detection reports it under the formats built - // on it, e.g. an svg comes back as `[text_file, xml, scalable_vector_ - // graphics]`, but there is no decoder of its own behind it yet. + // Classification only - reported under the formats built on it (an svg comes + // back as `[text_file, xml, scalable_vector_graphics]`), no decoder yet. // https://en.wikipedia.org/wiki/XML xml, }; @@ -174,9 +171,8 @@ enum class FileLocation { /// /// Declared, format-level support — an *upper bound*. A concrete file may still /// fail (corrupt, encrypted, an unsupported sub-variant); ask @ref DecodedFile -/// or @ref Document for the precise answer. The point of the static query is -/// the decisions a caller has to make *before* it holds a file, e.g. which -/// MIME types to advertise to the platform's file picker. +/// or @ref Document for that. This answers what a caller has to decide before +/// it holds a file, e.g. which MIME types to hand the platform's file picker. struct FileTypeCapabilities final { bool detect_by_content{}; ///< recognised from its bytes alone bool open{}; ///< a decoder exists; @ref odr::open can decode it @@ -293,10 +289,9 @@ class DecodedFile { /// @brief What can be done with this file. /// - /// Refines @ref capabilities_by_file_type with what is known about this - /// file. `edit`/`save`/`encrypt` are passed through from the format-level - /// declaration — resolving them exactly would mean decoding the document; - /// ask @ref Document::is_editable / @ref Document::is_savable for that. + /// Refines @ref capabilities_by_file_type with what is known about this file. + /// `edit`/`save`/`encrypt` stay as declared — ask @ref Document::is_editable + /// / @ref Document::is_savable for those. [[nodiscard]] FileTypeCapabilities capabilities() const; [[nodiscard]] bool is_text_file() const; diff --git a/src/odr/html.cpp b/src/odr/html.cpp index 7e1d252c5..8341b9c68 100644 --- a/src/odr/html.cpp +++ b/src/odr/html.cpp @@ -242,11 +242,9 @@ HtmlService html::translate(const DecodedFile &file, HtmlResourceLocator html::standard_resource_locator() { return [](const HtmlResource &resource, const HtmlConfig &config) -> HtmlResourceLocation { - if (!resource.is_accessible()) { - return resource.path(); - } - - if (config.embed_images && resource.type() == HtmlResourceType::image) { + // only an accessible image can be embedded; everything else is linked + if (resource.is_accessible() && config.embed_images && + resource.type() == HtmlResourceType::image) { return std::nullopt; } diff --git a/src/odr/html.hpp b/src/odr/html.hpp index f46538752..e361488f4 100644 --- a/src/odr/html.hpp +++ b/src/odr/html.hpp @@ -72,37 +72,24 @@ enum class HtmlTableGridlines { }; /// @brief Initial zoom of the emitted HTML on mobile (viewport meta tag). -/// -/// Desktop browsers ignore the viewport meta tag entirely. -/// -/// - `automatic`: per-content default — fixed-size paged content (PDF pages, -/// slides, drawings, images, text documents with page margins) uses -/// `fit_width`; reflowing content (spreadsheets, plain text) uses -/// `actual_size`. -/// - `fit_width`: the browser picks the initial zoom so the content's full -/// width fits the screen. -/// - `actual_size`: initial zoom locked to 100% (`initial-scale=1.0`). -/// - `none`: no viewport meta tag is written. +/// Desktop browsers ignore the tag entirely. enum class HtmlViewportMode { - automatic, - fit_width, - actual_size, - none, + automatic, ///< `fit_width` for fixed-size paged content (PDF pages, slides, + ///< drawings, images, text documents with page margins), + ///< `actual_size` for reflowing content (spreadsheets, text) + fit_width, ///< initial zoom fits the content's full width on screen + actual_size, ///< initial zoom locked to 100% (`initial-scale=1.0`) + none, ///< no viewport meta tag at all }; -/// @brief PDF text rendering mode. -/// -/// Selects how text is emitted in PDF→HTML output. -/// -/// - `dual_layer`: A visual layer (paint order, embedded PUA glyphs) and a -/// separate transparent selection/search layer (reading order, real Unicode). -/// Similar to pdf.js. No JavaScript required. -/// - `single_layer`: A single combined layer where every glyph is mapped to -/// Unicode via frequency analysis. Similar to pdf2htmlEX. No JavaScript -/// required. +/// @brief How text is emitted in PDF→HTML output. Neither mode needs +/// JavaScript. enum class PdfTextMode { - dual_layer, - single_layer, + dual_layer, ///< a visual layer (paint order, embedded PUA glyphs) plus a + ///< transparent selection layer (reading order, real Unicode), + ///< like pdf.js + single_layer, ///< one layer, every glyph mapped to Unicode by frequency + ///< analysis, like pdf2htmlEX }; /// @brief HTML configuration. @@ -165,17 +152,12 @@ struct HtmlConfig { // PDF text mode PdfTextMode pdf_text_mode{PdfTextMode::dual_layer}; - // `dual_layer`'s invisible selection-layer text is rendered in a local - // system font (tried in order; the first that resolves wins) rather than - // the embedded PDF font, so its natural width rarely matches the - // PDF-derived box width CSS `text-justify` is asked to fill (justify can - // only add spacing, never compress). - // `pdf_dual_layer_fallback_font_size_adjust` is applied as that @font-face's - // `size-adjust` (0-1, written out as a percent) to shrink the fallback font's - // metrics toward the PDF's, leaving less — ideally no — gap for justify to - // compress instead of stretch into. Safe to underestimate (justify then just - // spreads characters further; harmless on an invisible layer) but not to - // overestimate (the excess is clipped, not shrunk). + // `dual_layer` renders its invisible selection layer in a local system font + // (first of these that resolves), whose natural width rarely matches the + // PDF-derived box CSS justify has to fill — and justify can only add spacing. + // The size-adjust (0-1, written as the @font-face percent) shrinks the + // fallback's metrics toward the PDF's to close that gap. Safe to + // underestimate, not to overestimate: the excess is clipped, not shrunk. std::vector pdf_dual_layer_fallback_fonts{ "Arial", "Helvetica", "Liberation Sans", "DejaVu Sans", "Nimbus Sans"}; double pdf_dual_layer_fallback_font_size_adjust{0.5}; @@ -275,117 +257,53 @@ namespace html { HtmlResourceLocator standard_resource_locator(); -/// @brief Translates a decoded file to HTML. -/// -/// @param file Decoded file to translate. -/// @param cache_path Directory path for temporary output. -/// @param config Configuration for the HTML output. -/// @param logger Logger to use for logging. -/// @return HTML output. +/// @brief Translates a decoded file to HTML. `cache_path` is the directory +/// temporary output goes into. HtmlService translate(const DecodedFile &file, const std::string &cache_path, const HtmlConfig &config, const Logger &logger = Logger::null()); /// @brief Translates a text file to HTML. -/// -/// @param text_file Text file to translate. -/// @param cache_path Directory path for temporary output. -/// @param config Configuration for the HTML output. -/// @param logger Logger to use for logging. -/// @return HTML output. HtmlService translate(const TextFile &text_file, const std::string &cache_path, const HtmlConfig &config, const Logger &logger = Logger::null()); /// @brief Translates an image file to HTML. -/// -/// @param image_file Image file to translate. -/// @param cache_path Directory path for temporary output. -/// @param config Configuration for the HTML output. -/// @param logger Logger to use for logging. -/// @return HTML output. HtmlService translate(const ImageFile &image_file, const std::string &cache_path, const HtmlConfig &config, const Logger &logger = Logger::null()); -/// @brief Translates an archive to HTML. -/// -/// @param archive_file Archive file to translate. -/// @param cache_path Directory path for temporary output. -/// @param config Configuration for the HTML output. -/// @param logger Logger to use for logging. -/// @return HTML output. +/// @brief Translates an archive file to HTML. HtmlService translate(const ArchiveFile &archive_file, const std::string &cache_path, const HtmlConfig &config, const Logger &logger = Logger::null()); -/// @brief Translates a document to HTML. -/// -/// @param document_file Document file to translate. -/// @param cache_path Directory path for temporary output. -/// @param config Configuration for the HTML output. -/// @param logger Logger to use for logging. -/// @return HTML output. +/// @brief Translates a document file to HTML. HtmlService translate(const DocumentFile &document_file, const std::string &cache_path, const HtmlConfig &config, const Logger &logger = Logger::null()); /// @brief Translates a PDF file to HTML. -/// -/// @param pdf_file PDF file to translate. -/// @param cache_path Directory path for temporary output. -/// @param config Configuration for the HTML output. -/// @param logger Logger to use for logging. -/// @return HTML output. HtmlService translate(const PdfFile &pdf_file, const std::string &cache_path, const HtmlConfig &config, const Logger &logger = Logger::null()); /// @brief Translates a font file to HTML (a specimen page). -/// -/// @param font_file Font file to translate. -/// @param cache_path Directory path for temporary output. -/// @param config Configuration for the HTML output. -/// @param logger Logger to use for logging. -/// @return HTML output. HtmlService translate(const FontFile &font_file, const std::string &cache_path, const HtmlConfig &config, const Logger &logger = Logger::null()); /// @brief Translates a filesystem to HTML. -/// -/// @param filesystem Filesystem to translate. -/// @param cache_path Directory path for temporary output. -/// @param config Configuration for the HTML output. -/// @param logger Logger to use for logging. -/// @return HTML output. HtmlService translate(const Filesystem &filesystem, const std::string &cache_path, const HtmlConfig &config, const Logger &logger = Logger::null()); /// @brief Translates an archive to HTML. -/// -/// @param archive Archive to translate. -/// @param cache_path Directory path for temporary output. -/// @param config Configuration for the HTML output. -/// @param logger Logger to use for logging. -/// @return HTML output. HtmlService translate(const Archive &archive, const std::string &cache_path, const HtmlConfig &config, const Logger &logger = Logger::null()); /// @brief Translates a document to HTML. -/// -/// @param document Document to translate. -/// @param cache_path Directory path for temporary output. -/// @param config Configuration for the HTML output. -/// @param logger Logger to use for logging. -/// @return HTML output. HtmlService translate(const Document &document, const std::string &cache_path, const HtmlConfig &config, const Logger &logger = Logger::null()); -/// @brief Edits a document with a diff. -/// -/// @note The diff is generated by our JavaScript code in the browser. -/// -/// @param document Document to edit. -/// @param diff Diff to apply. -/// @param logger Logger to use for logging. +/// @brief Applies a diff to a document. The diff is what our JavaScript +/// produces in the browser. void edit(const Document &document, std::string_view diff, const Logger &logger = Logger::null()); diff --git a/src/odr/http_server.cpp b/src/odr/http_server.cpp index bdcc4705d..ae58e0687 100644 --- a/src/odr/http_server.cpp +++ b/src/odr/http_server.cpp @@ -18,19 +18,15 @@ namespace odr { class HttpServer::Impl : public std::enable_shared_from_this { public: - /// What a HttpServer holds: a second reference to the impl whose deleter - /// stops the server rather than destroying it. Running out of handles is what - /// has to stop a listen() in flight, and ~Impl cannot be that signal - - /// listen() keeps a reference of its own, so the impl outlives the handles - /// for as long as it serves. Destroying it is left to the reference captured - /// below. + /// What a HttpServer holds: a second reference whose deleter stops the server + /// rather than destroying it. ~Impl cannot be that signal - listen() keeps a + /// reference of its own, so the impl outlives the handles while it serves. static std::shared_ptr create(const Logger &logger) { std::shared_ptr owner = std::make_shared(logger); Impl *const impl = owner.get(); // shared_from_this() stays bound to the control block make_shared put - // there: a second one only takes weak_this over when it has expired. So - // listen() holds that reference, not one of these, which is the whole point + // there, so listen() holds `owner`'s reference, not one of these return std::shared_ptr{ impl, [owner = std::move(owner)](Impl * /*owner already has it*/) { owner->stop(); @@ -39,8 +35,7 @@ class HttpServer::Impl : public std::enable_shared_from_this { explicit Impl(const Logger &logger) : m_logger{logger}, m_server{std::make_shared()} { - // Set up exception handler to catch any internal httplib exceptions. - // This prevents crashes when exceptions occur during request processing. + // an exception escaping a handler tears down the process otherwise m_server->set_exception_handler([this](const httplib::Request & /*req*/, httplib::Response &res, const std::exception_ptr &ep) { @@ -62,52 +57,46 @@ class HttpServer::Impl : public std::enable_shared_from_this { res.set_content("Hello World!", "text/plain"); }); - m_server->Get("/file/" + std::string(prefix_pattern), - [this](const httplib::Request &req, httplib::Response &res) { - if (m_stopping.load(std::memory_order_acquire)) { - res.status = 503; - res.set_content("Service Unavailable", "text/plain"); - return; - } - serve_file(req, res); - }); + const auto file_handler = [this](const httplib::Request &req, + httplib::Response &res) { + if (m_stopping.load(std::memory_order_acquire)) { + res.status = 503; + res.set_content("Service Unavailable", "text/plain"); + return; + } + serve_file(req, res); + }; + m_server->Get("/file/" + std::string(prefix_pattern), file_handler); m_server->Get("/file/" + std::string(prefix_pattern) + "/(.*)", - [this](const httplib::Request &req, httplib::Response &res) { - if (m_stopping.load(std::memory_order_acquire)) { - res.status = 503; - res.set_content("Service Unavailable", "text/plain"); - return; - } - serve_file(req, res); - }); + file_handler); } ~Impl() { - // listen() holds a reference to the impl of its own, so this cannot run - // underneath an accept loop. stop() is still what tears the server down: it - // closes the socket, waits for any listen() to return and only then drops - // the httplib server, whose destructor joins the thread pool. m_content is - // destroyed after this body, i.e. after all of that. + // cannot run underneath an accept loop - listen() holds a reference of its + // own - but stop() is still what closes the socket and joins the pool, + // before m_content is destroyed after this body stop(); } - // Prevent copying - the lambdas capture 'this' so copying would be unsafe + // the handler lambdas capture `this` Impl(const Impl &) = delete; Impl &operator=(const Impl &) = delete; void serve_file(const httplib::Request &req, httplib::Response &res) { try { - std::string id = req.matches[1].str(); - std::string path = req.matches.size() > 1 ? req.matches[2].str() : ""; + const std::string id = req.matches[1].str(); + // the route without a trailing path has the one group + const std::string path = + req.matches.size() > 2 ? req.matches[2].str() : ""; std::unique_lock lock{m_mutex}; - auto it = m_content.find(id); + const auto it = m_content.find(id); if (it == m_content.end()) { ODR_ERROR(m_logger, "Content not found for ID: " << id); res.status = 404; return; } - auto [_, service] = it->second; + const HtmlService service = it->second.service; lock.unlock(); serve_file(res, service, path); @@ -132,14 +121,9 @@ class HttpServer::Impl : public std::enable_shared_from_this { ODR_VERBOSE(m_logger, "Serving file: " << path); - // Buffer content to avoid streaming issues on Android. - // Using ContentProviderWithoutLength (chunked transfer encoding) can cause - // SIGSEGV crashes in httplib::Server::write_response_core when: - // 1. The client disconnects during transfer - // 2. Exceptions are thrown during content generation - // 3. The server is stopped while requests are in-flight - // By buffering content first, we can handle errors gracefully and use - // Content-Length based responses which are more reliable. + // buffered rather than streamed: a chunked ContentProviderWithoutLength + // crashes httplib::Server::write_response_core when the client disconnects, + // the content generation throws, or the server stops mid-request try { std::ostringstream buffer; service.write(path, buffer); @@ -164,7 +148,7 @@ class HttpServer::Impl : public std::enable_shared_from_this { throw PrefixInUse(prefix); } - m_content.emplace(prefix, Content{prefix, std::move(service)}); + m_content.emplace(prefix, Content{std::move(service)}); } std::uint32_t bind(const std::string &host, const std::uint32_t port, @@ -181,18 +165,16 @@ class HttpServer::Impl : public std::enable_shared_from_this { } #ifdef _WIN32 - // Windows keeps cpp-httplib's defaults, which set SO_EXCLUSIVEADDRUSE - // alongside SO_REUSEADDR. The two flags mean the opposite of what they do - // below: there SO_REUSEADDR lets a second live socket take the endpoint - // over, and SO_EXCLUSIVEADDRUSE is what keeps it ours. Replacing that with - // the posix mapping would hand the port away, so Options does not apply. + // Options does not apply: Windows keeps cpp-httplib's + // SO_EXCLUSIVEADDRUSE defaults, where SO_REUSEADDR lets another live socket + // take the endpoint over and the posix mapping below would hand the port + // away static_cast(options); #else - // cpp-httplib's default sets SO_REUSEPORT where it exists and SO_REUSEADDR - // only otherwise, which is the wrong way round for a server that gets - // restarted: only SO_REUSEADDR lets a port held by TIME_WAIT sockets be - // bound again, while SO_REUSEPORT hands a second server a share of the - // connections instead. + // cpp-httplib defaults to SO_REUSEPORT where it exists, the wrong way round + // for a server that gets restarted: only SO_REUSEADDR rebinds a port held + // by TIME_WAIT sockets, SO_REUSEPORT shares connections with a second + // server instead. // socket_t is not in the httplib namespace in every version, hence auto m_server->set_socket_options([options](const auto sock) { constexpr int yes = 1; @@ -229,10 +211,9 @@ class HttpServer::Impl : public std::enable_shared_from_this { } void listen() { - // listen() blocks, so it runs on a thread of the caller's. A reference of - // its own keeps the impl - and with it the httplib server, the mutex and - // the condition variable below - alive for as long as the accept loop is on - // it, whatever the thread that owns the HttpServer does meanwhile. + // this blocks on a thread of the caller's; a reference of its own keeps the + // impl - server, mutex, condition variable - alive under the accept loop, + // whatever the thread owning the HttpServer does meanwhile const std::shared_ptr self = shared_from_this(); std::shared_ptr server; @@ -300,11 +281,10 @@ class HttpServer::Impl : public std::enable_shared_from_this { server = std::move(m_server); if (server != nullptr && m_listening > 0) { - // httplib::Server::stop() closes the listening socket, but only once - // listen_internal() has the accept loop up - and it asserts, then - // closes an already closed descriptor, if it runs a second time after - // that. A listen() that is still on its way there therefore has to be - // waited for, and httplib's own flag is the only thing to wait on. + // httplib::Server::stop() only closes the listening socket once the + // accept loop is up, and double-closes the descriptor if it runs again + // after that, so a listen() on its way there has to be waited for - + // httplib's own flag being the only thing to wait on while (m_listening > 0 && !server->is_running()) { m_listen_done.wait_for(lock, std::chrono::milliseconds{1}); } @@ -315,10 +295,8 @@ class HttpServer::Impl : public std::enable_shared_from_this { } } - // the accept loop stands on the server object and on everything the - // handlers capture, so neither may go before listen() has returned: - // dropping the server underneath it was the use after free, and the two - // then raced over the listening socket as well + // the accept loop stands on the server and on what the handlers capture, + // so neither may go before listen() has returned m_listen_done.wait(lock, [this] { return m_listening == 0; }); } @@ -351,17 +329,14 @@ class HttpServer::Impl : public std::enable_shared_from_this { // listen() calls in flight - 0 or 1 in any sane use std::size_t m_listening{0}; - // Flag to indicate server is shutting down - checked by handlers - // to reject new requests during shutdown. Atomic because they read it - // without the lock. + // rejects new requests; atomic because the handlers read it without the lock std::atomic m_stopping{false}; - // Whether bind() has taken a socket. listen() needs it because cpp-httplib - // will happily "serve" a server that never bound one. + // whether bind() has taken a socket - cpp-httplib will happily "serve" a + // server that never bound one bool m_bound{false}; struct Content { - std::string id; HtmlService service; }; diff --git a/src/odr/internal/abstract/archive.hpp b/src/odr/internal/abstract/archive.hpp index ecc8ca9c2..54b66eec3 100644 --- a/src/odr/internal/abstract/archive.hpp +++ b/src/odr/internal/abstract/archive.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include namespace odr::internal::abstract { diff --git a/src/odr/internal/abstract/document.hpp b/src/odr/internal/abstract/document.hpp index f11a4db77..40686990e 100644 --- a/src/odr/internal/abstract/document.hpp +++ b/src/odr/internal/abstract/document.hpp @@ -6,6 +6,7 @@ #include #include #include +#include namespace odr { class File; @@ -33,16 +34,11 @@ class Path; namespace odr::internal::abstract { class ReadableFilesystem; -} - -namespace odr::internal::abstract { class ElementAdapter; class TextRootAdapter; class SlideAdapter; class PageAdapter; class SheetAdapter; -class SheetColumnAdapter; -class SheetRowAdapter; class SheetCellAdapter; class MasterPageAdapter; class LineBreakAdapter; @@ -67,34 +63,22 @@ class Document { public: virtual ~Document() = default; - /// \return `true` if the document is editable in any way. + /// Editable in any way. [[nodiscard]] virtual bool is_editable() const noexcept = 0; - /// \param encrypted to ask for encrypted saves. - /// \return `true` if the document is is_savable. + /// Savable, @p encrypted to ask for an encrypted save. [[nodiscard]] virtual bool is_savable(bool encrypted) const noexcept = 0; - /// \param path the destination path. virtual void save(const Path &path) const = 0; - - /// \param path the destination path. - /// \param password the encryption password. virtual void save(const Path &path, const char *password) const = 0; - /// \return the type of the document. [[nodiscard]] virtual FileType file_type() const noexcept = 0; - - /// \return the type of the document. [[nodiscard]] virtual DocumentType document_type() const noexcept = 0; - /// \return the underlying filesystem of the document. [[nodiscard]] virtual std::shared_ptr as_filesystem() const noexcept = 0; - /// \return cursor to the root element of the document. [[nodiscard]] virtual ElementIdentifier root_element() const = 0; - - /// \return the element adapter for this document. [[nodiscard]] virtual const ElementAdapter *element_adapter() const = 0; }; diff --git a/src/odr/internal/abstract/font.hpp b/src/odr/internal/abstract/font.hpp index f7ba93a77..7a766ec83 100644 --- a/src/odr/internal/abstract/font.hpp +++ b/src/odr/internal/abstract/font.hpp @@ -8,14 +8,8 @@ namespace odr::internal::abstract { -/// @brief Read-only view over a font program, exposing the *facts* every -/// consumer needs while the raw glyph bytes pass through untouched. -/// -/// Per the "IR for facts, pass-through for glyphs" architecture this never -/// decompiles outlines: it reports counts / metrics / names and hands back the -/// original bytes. The embedded-font reverse map reads Unicode from it, the OTF -/// wrap synthesizes the SFNT skeleton from it, and the PUA re-encoder assigns -/// code points from its glyph count. +/// Read-only view over a font program: counts, metrics, names and character +/// maps. Outlines are never decompiled, the glyph bytes pass through untouched. class Font { public: virtual ~Font() = default; @@ -41,15 +35,13 @@ class Font { [[nodiscard]] virtual std::uint16_t advance_width(std::uint16_t glyph) const = 0; - /// The font's own forward map: Unicode code point -> glyph id, 0 (`.notdef`) - /// when unmapped. Seeds the specimen page and the PUA re-encode. + /// The font's own character map: code point -> glyph id, 0 (`.notdef`) when + /// unmapped. [[nodiscard]] virtual std::uint16_t glyph_for_code_point(char32_t code_point) const = 0; - /// The reverse map: glyph id -> Unicode code point, when the font's character - /// map reaches the glyph. This is the embedded-font reverse map used to - /// recover Unicode for a font with no usable `/ToUnicode` or `/Encoding`; - /// `nullopt` when no code point maps. + /// The reverse of that map, `nullopt` when no code point reaches @p glyph. + /// Recovers Unicode for a font without usable `/ToUnicode` or `/Encoding`. [[nodiscard]] virtual std::optional code_point_for_glyph(std::uint16_t glyph) const = 0; }; diff --git a/src/odr/internal/cfb/cfb_impl.cpp b/src/odr/internal/cfb/cfb_impl.cpp index 16ff813af..26375e91c 100644 --- a/src/odr/internal/cfb/cfb_impl.cpp +++ b/src/odr/internal/cfb/cfb_impl.cpp @@ -39,6 +39,14 @@ impl::CompoundFileEntry impl::parse_entry(std::istream &in) { namespace odr::internal::cfb::impl { std::string CompoundFileEntry::get_name() const { + // [MS-CFB] 2.6.1: `name_len` counts bytes including the terminating NUL and + // never exceeds the 64-byte name field. + if (name_len > sizeof(name)) { + throw CfbFileCorrupted(); + } + if (name_len < 2) { + return {}; + } return internal::util::string::c16str_to_string(name, name_len - 2); } @@ -109,47 +117,6 @@ void CompoundFileReader::read_file(std::istream &in, } } -void CompoundFileReader::visit_descendants( - std::istream &in, const CompoundFileEntry &entry, - const std::int32_t max_level, const EnumFilesCallback &callback) const { - const CompoundFileEntry child_entry = parse_entry(in, entry.child_id); - visit_descendants(in, child_entry, 0, max_level, std::u16string(), callback); -} - -void CompoundFileReader::visit_descendants( - std::istream &in, const CompoundFileEntry &entry, - const std::int32_t current_level, const std::int32_t max_level, - const std::u16string &dir, const EnumFilesCallback &callback) const { - if (max_level > 0 && current_level >= max_level) { - return; - } - - callback(entry, dir, current_level + 1); - - if (entry.child_id != NullId) { - const CompoundFileEntry child = parse_entry(in, entry.child_id); - - std::u16string new_dir = dir; - new_dir.append(entry.name, entry.name_len / 2); - visit_descendants(in, child, current_level + 1, max_level, new_dir, - callback); - } - - if (entry.left_sibling_id != NullId) { - const CompoundFileEntry left_sibling = - parse_entry(in, entry.left_sibling_id); - visit_descendants(in, left_sibling, current_level, max_level, dir, - callback); - } - - if (entry.right_sibling_id != NullId) { - const CompoundFileEntry right_sibling = - parse_entry(in, entry.right_sibling_id); - visit_descendants(in, right_sibling, current_level, max_level, dir, - callback); - } -} - void CompoundFileReader::read_stream(std::istream &in, const SectorOffset §or_offset, char *buffer, std::uint64_t length) const { diff --git a/src/odr/internal/cfb/cfb_impl.hpp b/src/odr/internal/cfb/cfb_impl.hpp index 9ce8b17c3..826f38b15 100644 --- a/src/odr/internal/cfb/cfb_impl.hpp +++ b/src/odr/internal/cfb/cfb_impl.hpp @@ -3,7 +3,6 @@ #include #include -#include #include namespace odr::internal::cfb::impl { @@ -68,10 +67,6 @@ CompoundFileEntry parse_entry(std::istream &in); class CompoundFileReader final { public: - using EnumFilesCallback = - std::function; - static constexpr auto MAGIC = "\xD0\xCF\x11\xE0\xA1\xB1\x1A\xE1"; explicit CompoundFileReader(std::istream &in, std::uint64_t file_size); @@ -100,10 +95,6 @@ class CompoundFileReader final { void read_file(std::istream &in, const CompoundFileEntry &entry, std::uint64_t offset, char *buffer, std::uint64_t len) const; - void visit_descendants(std::istream &in, const CompoundFileEntry &entry, - int max_level, - const EnumFilesCallback &callback) const; - private: struct SectorOffset final { Sector sector; @@ -112,12 +103,6 @@ class CompoundFileReader final { static constexpr Sector MaxSector = 0xFFFFFFFA; - // Enum entries with same level, including 'entry' itself - void visit_descendants(std::istream &in, const CompoundFileEntry &entry, - std::int32_t current_level, std::int32_t max_level, - const std::u16string &dir, - const EnumFilesCallback &callback) const; - void read_stream(std::istream &in, const SectorOffset §or_offset, char *buffer, std::uint64_t length) const; diff --git a/src/odr/internal/cfb/cfb_util.cpp b/src/odr/internal/cfb/cfb_util.cpp index f1ad9643b..00f6a2a87 100644 --- a/src/odr/internal/cfb/cfb_util.cpp +++ b/src/odr/internal/cfb/cfb_util.cpp @@ -1,5 +1,7 @@ #include +#include + #include #include @@ -172,6 +174,13 @@ std::optional Archive::Entry::child() const { m_path.join(RelPath(child.get_name()))); } +void Archive::Iterator::enter_(Entry entry) { + if (!m_visited.insert(entry.m_entry_id).second) { + throw CfbFileCorrupted(); + } + m_entry = std::move(entry); +} + void Archive::Iterator::dig_left_() { if (!m_entry.has_value()) { return; @@ -183,7 +192,7 @@ void Archive::Iterator::dig_left_() { break; } m_ancestors.push_back(*m_entry); - m_entry = left; + enter_(*left); } } @@ -194,7 +203,7 @@ void Archive::Iterator::next_() { if (const std::optional child = m_entry->child(); child.has_value()) { m_directories.push_back(*m_entry); - m_entry = child; + enter_(*child); dig_left_(); return; } @@ -208,7 +217,7 @@ void Archive::Iterator::next_flat_() { } if (const std::optional right = m_entry->right(); right.has_value()) { - m_entry = right; + enter_(*right); dig_left_(); return; } diff --git a/src/odr/internal/cfb/cfb_util.hpp b/src/odr/internal/cfb/cfb_util.hpp index 6e45d4246..ab9b78a3d 100644 --- a/src/odr/internal/cfb/cfb_util.hpp +++ b/src/odr/internal/cfb/cfb_util.hpp @@ -6,9 +6,13 @@ #include #include +#include #include #include +#include +#include #include +#include namespace odr::internal::cfb::impl { class CompoundFileReader; @@ -102,12 +106,17 @@ class Archive final : public std::enable_shared_from_this { std::optional m_entry; std::vector m_ancestors; std::vector m_directories; + std::set m_visited; Iterator() = default; - explicit Iterator(const Entry &root_entry) : m_entry{root_entry} { + explicit Iterator(const Entry &root_entry) { + enter_(root_entry); dig_left_(); } + /// Move onto a not-yet-visited entry; a repeat means a cyclic + /// child/sibling link ([MS-CFB] 2.6.4) that would never terminate. + void enter_(Entry entry); void dig_left_(); void next_(); void next_flat_(); diff --git a/src/odr/internal/common/filesystem.cpp b/src/odr/internal/common/filesystem.cpp index 92b47c3e7..ac8e81e5a 100644 --- a/src/odr/internal/common/filesystem.cpp +++ b/src/odr/internal/common/filesystem.cpp @@ -119,18 +119,19 @@ bool SystemFilesystem::copy(const AbsPath &from, const AbsPath &to) { std::shared_ptr SystemFilesystem::copy(const abstract::File &from, const AbsPath &to) { + // `create_file` and `open` translate `to` themselves const auto istream = from.stream(); - const auto ostream = create_file(to_system_path_(to)); + const auto ostream = create_file(to); util::stream::pipe(*istream, *ostream); - return open(to_system_path_(to)); + return open(to); } std::shared_ptr SystemFilesystem::copy(const std::shared_ptr from, const AbsPath &to) { - return copy(*from, to_system_path_(to)); + return copy(*from, to); } bool SystemFilesystem::move(const AbsPath &from, const AbsPath &to) { @@ -150,7 +151,7 @@ class VirtualFileWalker final : public abstract::FileWalker { VirtualFileWalker(const AbsPath &root, const Files &files) { for (const auto &[path, file] : files) { - if (path.ancestor_of(root)) { + if (path.descendant_of(root)) { m_files[path] = file; } } @@ -158,6 +159,13 @@ class VirtualFileWalker final : public abstract::FileWalker { m_iterator = std::begin(m_files); } + /// The iterator has to be re-seated into the copied map. + VirtualFileWalker(const VirtualFileWalker &other) : m_files{other.m_files} { + m_iterator = other.m_iterator == std::end(other.m_files) + ? std::end(m_files) + : m_files.find(other.m_iterator->first); + } + [[nodiscard]] std::unique_ptr clone() const override { return std::make_unique(*this); } diff --git a/src/odr/internal/common/path.cpp b/src/odr/internal/common/path.cpp index bb16aeb85..e9ccf8ae2 100644 --- a/src/odr/internal/common/path.cpp +++ b/src/odr/internal/common/path.cpp @@ -4,6 +4,20 @@ namespace odr::internal { +namespace { + +/// Whether @p path continues @p prefix at a component boundary, so that "/ab" +/// is not taken for a descendant of "/a". +bool has_path_prefix(const std::string &path, const std::string &prefix) { + if (!path.starts_with(prefix)) { + return false; + } + return prefix.empty() || prefix.back() == '/' || + path.size() == prefix.size() || path[prefix.size()] == '/'; +} + +} // namespace + Path::Path() noexcept : Path("") {} Path::Path(const char *c_string) : Path(std::string(c_string)) {} @@ -32,15 +46,10 @@ Path::Path(const std::filesystem::path &path) : Path(path.string()) {} void Path::parent_() { if (m_downwards > 0) { --m_downwards; - if (m_downwards == 0) { - if (m_absolute) { - m_path = "/"; - } else { - m_path = ""; - } + if (m_upwards + m_downwards == 0) { + m_path = m_absolute ? "/" : ""; } else { - const auto pos = m_path.rfind('/'); - m_path = m_path.substr(0, pos); + m_path = m_path.substr(0, m_path.rfind('/')); } } else if (!m_absolute) { if (m_upwards + m_downwards == 0) { @@ -129,7 +138,8 @@ bool Path::parent_of(const Path &b) const { throw std::invalid_argument("cannot compare absolute and relative path"); } // TODO we need to check upwards as well - return (m_downwards + 1 == b.m_downwards) && (b.m_path.rfind(m_path, 0) == 0); + return (m_downwards + 1 == b.m_downwards) && + has_path_prefix(b.m_path, m_path); } bool Path::ancestor_of(const Path &b) const { return b.descendant_of(*this); } @@ -139,7 +149,7 @@ bool Path::descendant_of(const Path &b) const { throw std::invalid_argument("cannot compare absolute and relative path"); } // TODO we need to check upwards as well - return (m_downwards < b.m_downwards) && (b.m_path.rfind(m_path, 0) == 0); + return (b.m_downwards < m_downwards) && has_path_prefix(m_path, b.m_path); } AbsPath Path::as_absolute() const & { return AbsPath(*this); } @@ -176,10 +186,8 @@ std::string Path::basename() const { } std::string Path::extension() const { - // The extension is the last dot-separated segment of the file name, without - // the leading dot (e.g. "a.b.ppt" -> "ppt"). Delegate to std::filesystem so - // edge cases (no extension, dot-files like ".bashrc") match the platform's - // definition; it returns ".ppt", so strip the leading dot. + // `std::filesystem` defines the edge cases (none, dot-files); it yields + // ".ppt", we want "ppt" std::string extension = path().extension().string(); if (!extension.empty() && extension.front() == '.') { extension.erase(extension.begin()); diff --git a/src/odr/internal/common/table_cursor.cpp b/src/odr/internal/common/table_cursor.cpp index 5a8d18852..7aca96278 100644 --- a/src/odr/internal/common/table_cursor.cpp +++ b/src/odr/internal/common/table_cursor.cpp @@ -7,7 +7,7 @@ namespace odr::internal { TableCursor::TableCursor() { m_sparse.emplace_back(); } -void TableCursor::add_column(const uint32_t repeat) noexcept { +void TableCursor::add_column(const std::uint32_t repeat) noexcept { m_column += repeat; } diff --git a/src/odr/internal/crypto/crypto_util.cpp b/src/odr/internal/crypto/crypto_util.cpp index 013da24a8..1588a1c70 100644 --- a/src/odr/internal/crypto/crypto_util.cpp +++ b/src/odr/internal/crypto/crypto_util.cpp @@ -188,14 +188,20 @@ std::string util::decrypt_aes_gcm(const std::string &key, const std::string &iv, const std::string &input) { // follows https://www.w3.org/TR/xmlenc-core1/#sec-AES-GCM - if (std::strncmp(iv.data(), input.data(), iv.size()) != 0) { + const std::size_t iv_size = iv.size(); + constexpr std::size_t mac_size = 16; + + // The input is IV || ciphertext || tag; anything shorter would wrap + // `cipher_size`. `memcmp`, not `strncmp` — both operands are binary. + if (input.size() < iv_size + mac_size) { + throw std::runtime_error("GCM input too short"); + } + if (std::memcmp(iv.data(), input.data(), iv_size) != 0) { throw std::runtime_error("IV mismatch"); } std::string result(input.size(), '\0'); - const std::size_t iv_size = iv.size(); - constexpr std::size_t mac_size = 16; const std::size_t cipher_size = input.size() - iv_size - mac_size; auto *message = reinterpret_cast(result.data()); const auto *mac = diff --git a/src/odr/internal/crypto/crypto_util.hpp b/src/odr/internal/crypto/crypto_util.hpp index 1d1bd52e6..20b335989 100644 --- a/src/odr/internal/crypto/crypto_util.hpp +++ b/src/odr/internal/crypto/crypto_util.hpp @@ -38,6 +38,9 @@ std::string decrypt_aes_cbc(const std::string &key, const std::string &iv, /// size). Needed by the PDF R 6 hardened-hash algorithm (ISO 32000-2 2.B). std::string encrypt_aes_cbc(const std::string &key, const std::string &iv, const std::string &input); +/// AES-GCM per XML Encryption 1.1 §5.2.4: @p input is `iv || ciphertext || +/// 16-byte tag` and must repeat @p iv. Throws if it does not, if @p input is +/// too short to hold both, or if the tag fails to verify. std::string decrypt_aes_gcm(const std::string &key, const std::string &iv, const std::string &input); std::string decrypt_triple_des(const std::string &key, const std::string &iv, diff --git a/src/odr/internal/file_type_table.cpp b/src/odr/internal/file_type_table.cpp index 483b79337..e7b1b5db0 100644 --- a/src/odr/internal/file_type_table.cpp +++ b/src/odr/internal/file_type_table.cpp @@ -11,10 +11,9 @@ using file_type_table::Row; using namespace std::string_view_literals; -// File extensions and MIME types accepted for each file type. The first entry -// of a list is the canonical one. Aliases have to stay unique across file -// types — the lookups take the first match and `odr_test` asserts that no -// extension or MIME type appears twice. +// Extensions and MIME types per file type, canonical one first. Aliases are +// unique across types — the lookups take the first match, `odr_test` asserts +// no alias appears twice. constexpr std::array odt_extensions{"odt"sv, "fodt"sv, "ott"sv, "odm"sv, "otm"sv}; @@ -79,9 +78,8 @@ constexpr std::array xlsx_mimetypes{ "application/vnd.ms-excel.template.macroEnabled.12"sv, }; -// `.xlsb` ships in an OOXML package but stores the workbook in binary parts -// instead of spreadsheetml, so it gets its own type rather than riding along -// with `xlsx` — the capability row is what tells a caller we cannot open it. +// `.xlsb` ships in an OOXML package but stores the workbook in binary parts, +// not spreadsheetml, so it is its own type with no capabilities. constexpr std::array xlsb_extensions{"xlsb"sv}; constexpr std::array xlsb_mimetypes{ "application/vnd.ms-excel.sheet.binary.macroEnabled.12"sv}; @@ -246,13 +244,12 @@ constexpr std::array avi_extensions{"avi"sv}; constexpr std::array avi_mimetypes{"video/x-msvideo"sv, "video/avi"sv, "video/msvideo"sv}; -// The single source of truth behind every public format lookup. `odr_test` -// asserts that it covers each `FileType` exactly once and that the capability -// bits agree with what the engines actually do. +// The single source of truth behind every public format lookup; `odr_test` +// asserts one row per `FileType` and capabilities that match the engines. // -// `decrypt` on the OOXML document types refers to a password-protected -// package, which is detected as `office_open_xml_encrypted` and decrypts into -// the type named here. ODF files decrypt in place and keep their type. +// `decrypt` on an OOXML document type means a password-protected package, +// detected as `office_open_xml_encrypted` and decrypting into the type named +// here. ODF files decrypt in place and keep their type. constexpr std::array table{ Row{FileType::unknown, "unknown"sv, @@ -505,11 +502,9 @@ constexpr std::array table{ DocumentType::unknown, {.detect_by_content = true, .open = true, .translate_html = true}}, - // Named but not decoded: `open` wraps the bytes without looking at them - // and `translate_html` hands them straight to the browser, in an `` - // for the images below and in a player for the audio and video after them. - // Nothing here reads a pixel or a sample - see the comment on these in - // `FileType`. + // Named but not decoded: `open` wraps the bytes and `translate_html` hands + // them to the browser in an `` or a player. Nothing reads a pixel or + // a sample. Row{FileType::webp, "webp"sv, webp_extensions, @@ -611,9 +606,9 @@ constexpr std::array table{ DocumentType::unknown, {.detect_by_content = true, .open = true, .translate_html = true}}, - // Named but not decoded, like the images above. `translate_html` says the - // image page is written and the data url is labelled with the type below, - // not that every browser paints it - that is already true of tiff and heif. + // Named but not decoded, like the images above. `translate_html` means the + // image page is written and the data url labelled, not that every browser + // paints it. Row{FileType::scalable_vector_graphics, "svg"sv, svg_extensions, @@ -680,7 +675,8 @@ template const Row *find_by_alias(const std::string_view needle, Projection list) noexcept { const auto it = std::ranges::find_if(table, [&](const Row &row) { - return std::ranges::find(list(row), needle) != std::ranges::end(list(row)); + const auto aliases = list(row); + return std::ranges::find(aliases, needle) != std::ranges::end(aliases); }); return it == std::ranges::end(table) ? nullptr : &*it; } diff --git a/src/odr/internal/font/cff_builder.hpp b/src/odr/internal/font/cff_builder.hpp index fc554bd52..50f8da694 100644 --- a/src/odr/internal/font/cff_builder.hpp +++ b/src/odr/internal/font/cff_builder.hpp @@ -21,17 +21,12 @@ struct BuilderGlyph { /// browser) needs: Header, Name INDEX, Top DICT (FontBBox + /// charset/CharStrings/Private offsets), String INDEX (every glyph name, SID /// 391+), an empty Global Subr INDEX, the CharStrings INDEX, a format-0 charset -/// and a Private DICT -/// (`defaultWidthX`/`nominalWidthX`). Glyph 0 is the implicit `.notdef`; the -/// caller orders @p glyphs so glyph 0 is `.notdef`. +/// and a Private DICT (`defaultWidthX`/`nominalWidthX`). The caller orders +/// @p glyphs so glyph 0 is the implicit `.notdef`. /// -/// This is the assembly target for the Type1 -> CFF path: the translated Type2 -/// charstrings go in here, the result feeds `CffFont` + `wrap_to_otf`. No -/// `FontMatrix` is emitted, so the font is 1000 -/// units/em (the Type1 default); a non-default matrix is a follow-up. -/// -/// Offsets in the Top DICT use the fixed-width 5-byte integer form so the -/// layout resolves in a single pass. +/// No `FontMatrix` is emitted, so the font is 1000 units/em (the Type1 +/// default); a non-default matrix is a follow-up. Top DICT offsets use the +/// fixed-width 5-byte integer form so the layout resolves in a single pass. [[nodiscard]] std::string build_cff(std::string_view name, const std::vector &glyphs, double default_width, double nominal_width, diff --git a/src/odr/internal/font/cff_font.cpp b/src/odr/internal/font/cff_font.cpp index e890df3ef..c66a2fa2d 100644 --- a/src/odr/internal/font/cff_font.cpp +++ b/src/odr/internal/font/cff_font.cpp @@ -4,10 +4,12 @@ #include #include +#include +#include #include #include -#include #include +#include #include #include #include @@ -33,6 +35,8 @@ enum Operator : std::uint16_t { op_font_bbox = 5, op_ros = 1230, op_charstring_type = 1206, + op_fd_array = 1236, + op_fd_select = 1237, }; /// Number-operand byte markers shared by DICT data and Type2 charstrings @@ -96,6 +100,12 @@ namespace bs = util::byte_string; /// stay exact within double range (CFF integers fit). using Dict = std::map>; +/// A DICT operand as an FWord. Clamping keeps an out-of-range operand out of +/// the undefined double -> int16 conversion. +[[nodiscard]] std::int16_t to_fword(const double value) { + return static_cast(std::clamp(value, -32768.0, 32767.0)); +} + /// Parse a CFF DICT occupying the byte range [begin, end) of @p d. [[nodiscard]] Dict parse_dict(const std::string_view d, const std::uint32_t begin, @@ -181,28 +191,28 @@ enum PredefinedCharset : std::uint32_t { /// Predefined Expert charset: glyph -> SID (Adobe TN #5176 Appendix C). The /// ISOAdobe charset is the identity (SID == GID) so it needs no table. -constexpr std::uint16_t expert_charset[] = { - 0, 1, 229, 230, 231, 232, 233, 234, 235, 236, 237, 238, 13, 14, - 15, 99, 239, 240, 241, 242, 243, 244, 245, 246, 247, 248, 27, 28, - 249, 250, 251, 252, 253, 254, 255, 256, 257, 258, 259, 260, 261, 262, - 263, 264, 265, 266, 109, 110, 267, 268, 269, 270, 271, 272, 273, 274, - 275, 276, 277, 278, 279, 280, 281, 282, 283, 284, 285, 286, 287, 288, - 289, 290, 291, 292, 293, 294, 295, 296, 297, 298, 299, 300, 301, 302, - 303, 304, 305, 306, 307, 308, 309, 310, 311, 312, 313, 314, 315, 316, - 317, 318, 158, 155, 163, 319, 320, 321, 322, 323, 324, 325, 326, 150, - 164, 169, 327, 328, 329, 330, 331, 332, 333, 334, 335, 336, 337, 338, - 339, 340, 341, 342, 343, 344, 345, 346, 347, 348, 349, 350, 351, 352, - 353, 354, 355, 356, 357, 358, 359, 360, 361, 362, 363, 364, 365, 366, - 367, 368, 369, 370, 371, 372, 373, 374, 375, 376, 377, 378}; +constexpr auto expert_charset = std::to_array( + {0, 1, 229, 230, 231, 232, 233, 234, 235, 236, 237, 238, 13, 14, + 15, 99, 239, 240, 241, 242, 243, 244, 245, 246, 247, 248, 27, 28, + 249, 250, 251, 252, 253, 254, 255, 256, 257, 258, 259, 260, 261, 262, + 263, 264, 265, 266, 109, 110, 267, 268, 269, 270, 271, 272, 273, 274, + 275, 276, 277, 278, 279, 280, 281, 282, 283, 284, 285, 286, 287, 288, + 289, 290, 291, 292, 293, 294, 295, 296, 297, 298, 299, 300, 301, 302, + 303, 304, 305, 306, 307, 308, 309, 310, 311, 312, 313, 314, 315, 316, + 317, 318, 158, 155, 163, 319, 320, 321, 322, 323, 324, 325, 326, 150, + 164, 169, 327, 328, 329, 330, 331, 332, 333, 334, 335, 336, 337, 338, + 339, 340, 341, 342, 343, 344, 345, 346, 347, 348, 349, 350, 351, 352, + 353, 354, 355, 356, 357, 358, 359, 360, 361, 362, 363, 364, 365, 366, + 367, 368, 369, 370, 371, 372, 373, 374, 375, 376, 377, 378}); /// Predefined ExpertSubset charset: glyph -> SID (Adobe TN #5176 Appendix C). -constexpr std::uint16_t expert_subset_charset[] = { - 0, 1, 231, 232, 235, 236, 237, 238, 13, 14, 15, 99, 239, 240, 241, - 242, 243, 244, 245, 246, 247, 248, 27, 28, 249, 250, 251, 253, 254, 255, - 256, 257, 258, 259, 260, 261, 262, 263, 264, 265, 266, 109, 110, 267, 268, - 269, 270, 272, 300, 301, 302, 305, 314, 315, 158, 155, 163, 320, 321, 322, - 323, 324, 325, 326, 150, 164, 169, 327, 328, 329, 330, 331, 332, 333, 334, - 335, 336, 337, 338, 339, 340, 341, 342, 343, 344, 345, 346}; +constexpr auto expert_subset_charset = std::to_array( + {0, 1, 231, 232, 235, 236, 237, 238, 13, 14, 15, 99, 239, 240, 241, + 242, 243, 244, 245, 246, 247, 248, 27, 28, 249, 250, 251, 253, 254, 255, + 256, 257, 258, 259, 260, 261, 262, 263, 264, 265, 266, 109, 110, 267, 268, + 269, 270, 272, 300, 301, 302, 305, 314, 315, 158, 155, 163, 320, 321, 322, + 323, 324, 325, 326, 150, 164, 169, 327, 328, 329, 330, 331, 332, 333, 334, + 335, 336, 337, 338, 339, 340, 341, 342, 343, 344, 345, 346}); } // namespace @@ -237,6 +247,11 @@ std::vector CffFont::read_index(const std::uint32_t offset, for (std::uint16_t i = 1; i <= count; ++i) { const std::uint32_t next = read_be(d, offset_array + i * off_size, off_size); + // The offset array is non-decreasing (Adobe TN #5176 §5); otherwise the + // member length would wrap. + if (next < prev) { + throw std::runtime_error("cff: non-monotonic INDEX offsets"); + } members.push_back({data_base + prev, next - prev}); prev = next; } @@ -295,10 +310,8 @@ void CffFont::parse_top_dict(const Range top_dict) { if (const auto it = dict.find(op_font_bbox); it != dict.end() && it->second.size() == 4) { - m_bbox = {static_cast(it->second[0]), - static_cast(it->second[1]), - static_cast(it->second[2]), - static_cast(it->second[3])}; + m_bbox = {to_fword(it->second[0]), to_fword(it->second[1]), + to_fword(it->second[2]), to_fword(it->second[3])}; } if (const auto it = dict.find(op_char_strings); it != dict.end()) { @@ -311,7 +324,15 @@ void CffFont::parse_top_dict(const Range top_dict) { it != dict.end() && it->second.size() == 2) { const auto size = static_cast(it->second[0]); const auto offset = static_cast(it->second[1]); - parse_private_dict({offset, size}); + m_widths = parse_private_dict({offset, size}); + } + + // A CID-keyed font keeps its Private DICTs per FD, not in the Top DICT. + if (const auto it = dict.find(op_fd_array); it != dict.end()) { + parse_fd_array(static_cast(it->second.at(0))); + } + if (const auto it = dict.find(op_fd_select); it != dict.end()) { + parse_fd_select(static_cast(it->second.at(0))); } // charset: an offset past the predefined ids (0/1/2) is a custom charset; @@ -339,33 +360,87 @@ void CffFont::load_predefined_charset(const std::uint32_t id) { } return; } - const std::uint16_t *table = nullptr; - std::size_t size = 0; - if (id == predefined_charset_expert) { - table = expert_charset; - size = std::size(expert_charset); - } else { // predefined_charset_expert_subset - table = expert_subset_charset; - size = std::size(expert_subset_charset); - } - for (std::uint16_t gid = 1; gid < glyphs && gid < size; ++gid) { + const std::span table = + id == predefined_charset_expert + ? std::span(expert_charset) + : std::span(expert_subset_charset); + for (std::uint16_t gid = 1; gid < glyphs && gid < table.size(); ++gid) { m_charset[gid] = table[gid]; } } -void CffFont::parse_private_dict(const Range private_dict) { +CffFont::Widths CffFont::parse_private_dict(const Range private_dict) const { + Widths widths; if (private_dict.length == 0) { - return; + return widths; } const std::string_view d{m_data}; const Dict dict = parse_dict(d, private_dict.offset, private_dict.offset + private_dict.length); if (const auto it = dict.find(op_default_width_x); it != dict.end()) { - m_default_width = it->second.at(0); + widths.default_width = it->second.at(0); } if (const auto it = dict.find(op_nominal_width_x); it != dict.end()) { - m_nominal_width = it->second.at(0); + widths.nominal_width = it->second.at(0); } + return widths; +} + +void CffFont::parse_fd_array(const std::uint32_t offset) { + const std::string_view d{m_data}; + std::uint32_t end = 0; + for (const Range font_dict : read_index(offset, end)) { + const Dict dict = + parse_dict(d, font_dict.offset, font_dict.offset + font_dict.length); + Widths widths; + if (const auto it = dict.find(op_private); + it != dict.end() && it->second.size() == 2) { + widths = parse_private_dict({static_cast(it->second[1]), + static_cast(it->second[0])}); + } + m_fd_widths.push_back(widths); + } +} + +void CffFont::parse_fd_select(const std::uint32_t offset) { + const std::string_view d{m_data}; + const std::uint16_t glyphs = glyph_count(); + const std::uint8_t format = u8(d, offset); + + if (format == 0) { + m_fd_select.reserve(glyphs); + for (std::uint16_t gid = 0; gid < glyphs; ++gid) { + m_fd_select.push_back(u8(d, offset + 1 + gid)); + } + return; + } + if (format != 3) { + throw std::runtime_error("cff: unknown FDSelect format"); + } + + // format 3: ranges of [first, next first) sharing one FD, then a sentinel + const auto ranges = static_cast(read_be(d, offset + 1, 2)); + m_fd_select.assign(glyphs, 0); + for (std::uint16_t i = 0; i < ranges; ++i) { + const std::uint32_t entry = offset + 3 + 3 * i; + const auto first = static_cast(read_be(d, entry, 2)); + const std::uint8_t fd = u8(d, entry + 2); + const auto next = static_cast(read_be(d, entry + 3, 2)); + for (std::uint32_t gid = first; gid < next && gid < glyphs; ++gid) { + m_fd_select[gid] = fd; + } + } +} + +const CffFont::Widths & +CffFont::widths_for_glyph(const std::uint16_t glyph) const { + if (!m_fd_widths.empty()) { + const std::size_t fd = glyph < m_fd_select.size() ? m_fd_select[glyph] : 0; + if (fd < m_fd_widths.size()) { + return m_fd_widths[fd]; + } + } + return m_widths; } void CffFont::parse_charset(const std::uint32_t offset) { @@ -510,11 +585,13 @@ bool CffFont::symbolic() const noexcept { FontBBox CffFont::bounding_box() const noexcept { return m_bbox; } std::uint16_t CffFont::advance_width(const std::uint16_t glyph) const { - if (const std::optional width = charstring_width(glyph); - width.has_value()) { - return static_cast(m_nominal_width + *width); - } - return static_cast(m_default_width); + const Widths &widths = widths_for_glyph(glyph); + const std::optional width = charstring_width(glyph); + const double advance = + width.has_value() ? widths.nominal_width + *width : widths.default_width; + // An advance is a uFWord; clamping keeps a hostile Private DICT out of the + // undefined double -> uint16 conversion. + return static_cast(std::clamp(advance, 0.0, 65535.0)); } std::uint16_t CffFont::glyph_for_code_point(const char32_t code_point) const { diff --git a/src/odr/internal/font/cff_font.hpp b/src/odr/internal/font/cff_font.hpp index 53249f07f..609a8e100 100644 --- a/src/odr/internal/font/cff_font.hpp +++ b/src/odr/internal/font/cff_font.hpp @@ -49,8 +49,7 @@ class CffFont final : public abstract::Font { [[nodiscard]] bool is_cid_keyed() const noexcept; /// The glyph's PostScript name (non-CID fonts), empty when unresolved (a - /// CID-keyed font, an out-of-range glyph, or a standard-string SID until the - /// standard-strings table lands — see the .cpp TODO). + /// CID-keyed font, an out-of-range glyph, or an unknown SID). [[nodiscard]] std::string glyph_name(std::uint16_t glyph) const; /// charset glyph -> CID (CID-keyed fonts), `0` when out of range or not @@ -69,9 +68,24 @@ class CffFont final : public abstract::Font { std::uint32_t length{}; }; + /// The two Private DICT entries a charstring's width is resolved against + /// (Adobe TN #5177 "width"). + struct Widths { + double default_width{}; + double nominal_width{}; + }; + void parse(); void parse_top_dict(Range top_dict); - void parse_private_dict(Range private_dict); + [[nodiscard]] Widths parse_private_dict(Range private_dict) const; + /// Parse `/FDArray`'s per-FD Private DICTs and `/FDSelect` (CID-keyed fonts; + /// Adobe TN #5176 §19). Without these a CID font resolves every width against + /// a `nominalWidthX` of 0. + void parse_fd_array(std::uint32_t offset); + void parse_fd_select(std::uint32_t offset); + /// The Private DICT widths governing @p glyph — its FD's for a CID-keyed + /// font, the Top DICT's otherwise. + [[nodiscard]] const Widths &widths_for_glyph(std::uint16_t glyph) const; void parse_charset(std::uint32_t offset); /// Materialize a predefined charset (id 0 ISOAdobe / 1 Expert / 2 /// ExpertSubset) into `m_charset`, used when `/charset` is a predefined id or @@ -100,8 +114,9 @@ class CffFont final : public abstract::Font { std::vector m_strings; // String INDEX members (SID 391+) std::vector m_charset; // glyph -> SID (or CID, CID-keyed) - double m_default_width{}; - double m_nominal_width{}; + Widths m_widths; // Top DICT Private DICT + std::vector m_fd_widths; // per FDArray entry, CID-keyed only + std::vector m_fd_select; // glyph -> FDArray index }; } // namespace odr::internal::font::cff diff --git a/src/odr/internal/font/cff_transform.cpp b/src/odr/internal/font/cff_transform.cpp index 34c456531..13a4dc16f 100644 --- a/src/odr/internal/font/cff_transform.cpp +++ b/src/odr/internal/font/cff_transform.cpp @@ -99,24 +99,9 @@ std::string cff::wrap_to_otf(const CffFont &font, const std::map &extra) { const std::uint16_t glyphs = font.glyph_count(); - // The uniform PUA re-encode: pua_code_point(glyph) -> glyph over every glyph. - // Glyphs past the 6400-slot BMP PUA overflow into Supplementary PUA-A, and - // serialize_cmap emits a format-12 subtable to cover them. - std::map pua; - for (std::uint16_t glyph = 0; glyph < glyphs; ++glyph) { - pua[pua_code_point(glyph)] = glyph; - } - // Real-Unicode entries: caller guarantees BMP, non-PUA keys, so these never - // collide with the PUA range filled above. A glyph id the font does not have - // is dropped: `glyph_for_code` can fall back to "code as GID" (ISO 32000-1 - // 9.6.6.4) and yield an out-of-range index, and a single cmap reference past - // `numGlyphs` makes the OTS sanitizer reject the *entire* font (so every - // glyph would render as a tofu box, not just the unmappable code). - for (const auto &[code, glyph] : extra) { - if (glyph < glyphs) { - pua[code] = glyph; - } - } + // Glyphs past the 6400-slot BMP PUA overflow into Supplementary PUA-A, which + // serialize_cmap covers with a format-12 subtable. + const std::map pua = pua_cmap(glyphs, extra); std::uint16_t advance_width_max = 0; for (std::uint16_t glyph = 0; glyph < glyphs; ++glyph) { diff --git a/src/odr/internal/font/cff_transform.hpp b/src/odr/internal/font/cff_transform.hpp index 2c85593a7..838e5d04a 100644 --- a/src/odr/internal/font/cff_transform.hpp +++ b/src/odr/internal/font/cff_transform.hpp @@ -14,21 +14,11 @@ class CffFont; /// sanitizer) require, so this synthesizes the skeleton — `head` / `hhea` / /// `maxp` (v0.5) / `hmtx` / `name` / `post` / `OS/2` — from the /// `abstract::Font` facts and embeds the original CFF verbatim as the `CFF ` -/// table (pass-through, no outline interpretation). The `cmap` is the **uniform -/// PUA re-encode**: `pua_code_point(glyph) -> glyph` over every -/// glyph, so the font renders every glyph — including charset-unreachable ones -/// — when loaded via `@font-face`, matching the PUA code points the PDF HTML -/// layer emits. +/// table (pass-through, no outline interpretation). /// -/// @p extra adds real-Unicode -> glyph entries alongside the PUA range, so a -/// run whose codes map 1:1 to those scalars can render the *real* Unicode -/// directly (the HTML layer then collapses its dual selectable/visible spans -/// into one). Keys must be in the BMP and outside the PUA (`U+E000..U+F8FF`); -/// the caller guarantees this. The PUA range is always kept as a fallback. -/// -/// Reuses the `sfnt_transform` serializers (`build_sfnt`, `serialize_cmap`, -/// `serialize_post`, `serialize_os2`). Throws `std::runtime_error` if the glyph -/// count exceeds the BMP PUA capacity (6400). +/// The `cmap` is `pua_cmap(glyph_count, extra)`, so the font renders every +/// glyph — including charset-unreachable ones — at the PUA code points the PDF +/// HTML layer emits. [[nodiscard]] std::string wrap_to_otf(const CffFont &font, const std::map &extra = {}); diff --git a/src/odr/internal/font/sfnt_font.cpp b/src/odr/internal/font/sfnt_font.cpp index da0d820a2..bf598e066 100644 --- a/src/odr/internal/font/sfnt_font.cpp +++ b/src/odr/internal/font/sfnt_font.cpp @@ -18,9 +18,7 @@ namespace bs = util::byte_string; namespace { -// SFNT enumerations (OpenType spec). Values are the on-disk codes; casting a -// raw `u16` to one and switching/comparing keeps the magic numbers in one -// place. +// The enumerators below are the on-disk codes (OpenType spec). /// `cmap`/`name` platform IDs. enum class PlatformId : std::uint16_t { @@ -203,8 +201,7 @@ void SfntFont::read_directory(const std::string_view sfnt) { : FontFormat::truetype; const std::uint16_t num_tables = bs::read_u16_be(sfnt.substr(4)); - // The offset table is 12 bytes (sfntVersion, numTables, then the three search - // hints); each of the `num_tables` directory entries is 16 bytes: tag(4), + // Past the 12 byte offset table, each directory entry is 16 bytes: tag(4), // checkSum(4), offset(4), length(4). for (std::uint16_t i = 0; i < num_tables; ++i) { const std::size_t entry = 12 + static_cast(i) * 16; @@ -309,9 +306,6 @@ void SfntFont::read_cmap_subtable(const std::string_view s) { m_cmap[code] = glyph; }; - // Every subtable format has a fixed-layout header, so each field is read at - // its known offset (matching the rest of this file). Format 4's arrays are - // variable-length, but each one's offset is a fixed function of segCount. const auto read_u16_vector = [](const std::string_view v, const std::size_t count) { std::vector out; @@ -330,11 +324,10 @@ void SfntFont::read_cmap_subtable(const std::string_view s) { } break; } - case CmapFormat::segment_mapping: { // segment mapping to delta values - // format(0), length(2), language(4), segCountX2(6), then searchRange(8), - // entrySelector(10), rangeShift(12). The four parallel segs-sized arrays - // follow: endCode(14), reservedPad, startCode, idDelta, idRangeOffset, each - // starting at a fixed offset once segCount is known. + case CmapFormat::segment_mapping: { + // format(0), length(2), language(4), segCountX2(6), 3 search hints(8..12), + // then the parallel segs-sized arrays endCode(14), reservedPad, startCode, + // idDelta, idRangeOffset. const std::uint16_t length = bs::read_u16_be(s.substr(2)); const std::size_t segs = bs::read_u16_be(s.substr(6)) / 2U; const std::vector end_codes = @@ -345,9 +338,8 @@ void SfntFont::read_cmap_subtable(const std::string_view s) { read_u16_vector(s.substr(16 + 4 * segs), segs); const std::vector id_range_offsets = read_u16_vector(s.substr(16 + 6 * segs), segs); - // Whatever remains of the subtable is the glyphIdArray that non-zero - // idRangeOffsets index into; preload it so the inner loop is a plain - // lookup. The header up to this point is 16 + 8*segs bytes. + // What remains past the 16 + 8*segs byte header is the glyphIdArray that + // non-zero idRangeOffsets index into. const std::size_t header = 16 + 8 * segs; if (length < header) { throw std::runtime_error("sfnt: cmap format 4 subtable too short"); @@ -505,22 +497,16 @@ std::string SfntFont::write() const { } tables.emplace_back("cmap", serialize_cmap(m_cmap)); - // A `post` table is required by OTS; PDF-embedded TrueType fonts often omit - // it. Synthesize a minimal one so the browser accepts the `@font-face`. + // OTS rejects a font missing `post` / `name` / `OS/2`, and PDF-embedded + // TrueType routinely omits all three; synthesize the missing ones so the + // browser accepts the `@font-face`. build_sfnt sorts the directory, so the + // insertion order here does not matter. if (!m_tables.contains("post")) { tables.emplace_back("post", serialize_post()); } - - // `name` is likewise required by OTS and likewise often omitted from - // TrueType subsets. Synthesize a minimal one (falls back to "ODR Font" when - // the font carries no name at all). if (!m_tables.contains("name")) { tables.emplace_back("name", serialize_name(m_name)); } - - // `OS/2` is likewise required by OTS and likewise often omitted. Synthesize - // it from the cmap bounds and bounding box (build_sfnt sorts the directory, - // so the insertion order here does not matter). if (!m_tables.contains("OS/2")) { std::uint16_t first_char = 0; std::uint16_t last_char = 0; diff --git a/src/odr/internal/font/sfnt_transform.cpp b/src/odr/internal/font/sfnt_transform.cpp index d6cae88e0..1b192bc3a 100644 --- a/src/odr/internal/font/sfnt_transform.cpp +++ b/src/odr/internal/font/sfnt_transform.cpp @@ -150,12 +150,11 @@ font::build_sfnt(const std::uint32_t sfnt_version, return out; } -/// Format-12 `cmap` subtable (segmented coverage): sequential map groups over -/// the full Unicode range, each `[startCharCode, endCharCode]` mapping to -/// `startGlyphID + (code - startCharCode)`. Used when the map reaches beyond -/// the BMP (glyphs overflowing into Supplementary PUA-A), which format 4 cannot -/// express. Wrapped in a (Windows, Unicode full repertoire) encoding record. -static std::string +namespace { + +/// Format-12 `cmap` subtable (segmented coverage), in a (Windows, Unicode full +/// repertoire) encoding record — what format 4 cannot express. +std::string serialize_cmap_format12(const std::map &map) { struct Group { std::uint32_t start_code; @@ -196,6 +195,8 @@ serialize_cmap_format12(const std::map &map) { return cmap; } +} // namespace + std::string font::serialize_cmap(const std::map &map) { // Format 4 tops out at the BMP; a map that overflows into the Supplementary // PUA needs format 12's 32-bit code ranges instead. @@ -375,29 +376,32 @@ std::string font::serialize_os2(const std::uint16_t units_per_em, return os2; } -void font::reencode_to_pua(sfnt::SfntFont &font, - const std::map &extra) { +std::map +font::pua_cmap(const std::uint16_t glyph_count, + const std::map &extra) { // A uint16 glyph id always fits: `pua_code_point` maps the BMP PUA first // (6400 slots) then overflows into Supplementary PUA-A, whose combined // `pua_capacity` (71934) exceeds any 16-bit glyph count. static_assert(std::numeric_limits::max() < pua_capacity); std::map map; - for (std::uint16_t glyph = 0; glyph < font.glyph_count(); ++glyph) { + for (std::uint16_t glyph = 0; glyph < glyph_count; ++glyph) { map[pua_code_point(glyph)] = glyph; } - // Real-Unicode entries: caller guarantees BMP, non-PUA keys, so these never - // collide with the PUA range filled above. A glyph id the font does not have - // is dropped: `glyph_for_code` can fall back to "code as GID" (ISO 32000-1 - // 9.6.6.4) and yield an out-of-range index, and a single cmap reference past - // `numGlyphs` makes the OTS sanitizer reject the *entire* font (so every - // glyph would render as a tofu box, not just the unmappable code). + // The caller guarantees BMP, non-PUA keys, so these never collide with the + // range filled above. An out-of-range glyph is dropped: `glyph_for_code` can + // fall back to "code as GID" (ISO 32000-1 9.6.6.4). for (const auto &[code, glyph] : extra) { - if (glyph < font.glyph_count()) { + if (glyph < glyph_count) { map[code] = glyph; } } - font.set_cmap(std::move(map)); + return map; +} + +void font::reencode_to_pua(sfnt::SfntFont &font, + const std::map &extra) { + font.set_cmap(pua_cmap(font.glyph_count(), extra)); } } // namespace odr::internal diff --git a/src/odr/internal/font/sfnt_transform.hpp b/src/odr/internal/font/sfnt_transform.hpp index 31a0a9b1d..31b84d5f1 100644 --- a/src/odr/internal/font/sfnt_transform.hpp +++ b/src/odr/internal/font/sfnt_transform.hpp @@ -12,68 +12,52 @@ namespace sfnt { class SfntFont; } -/// The deterministic Private Use Area code point that the uniform -/// re-encode assigns to glyph @p glyph: -/// `U+E000 + glyph` in the BMP PUA. Every consumer (the specimen page, the PDF -/// `@font-face` emission) derives the displayed code point from this — no -/// per-font table needed. +/// The deterministic Private Use Area code point the uniform re-encode assigns +/// to glyph @p glyph: `U+E000 + glyph` while the BMP PUA lasts (6400 slots), +/// then Supplementary PUA-A from `U+F0000`. Every consumer derives the +/// displayed code point from this — no per-font table needed. [[nodiscard]] char32_t pua_code_point(std::uint16_t glyph) noexcept; +/// The uniform PUA re-encode as a `cmap` model: `pua_code_point(glyph) -> +/// glyph` for every glyph below @p glyph_count, plus @p extra's real-Unicode +/// entries alongside it (keys must be in the BMP and outside `U+E000..U+F8FF` +/// so they never shadow a glyph's own PUA code point; the caller guarantees +/// this). An @p extra entry naming a glyph the font does not have is dropped — +/// a single `cmap` reference past `numGlyphs` makes the OTS sanitizer reject +/// the *entire* font, so every glyph would render as tofu. +[[nodiscard]] std::map +pua_cmap(std::uint16_t glyph_count, + const std::map &extra = {}); + /// Serialize an SFNT from its tables, computing the table directory, per-table -/// checksums and `head.checkSumAdjustment`, and return the assembled bytes. -/// @p tables need not be sorted; a `head` table is patched in place with the -/// final adjustment. -/// -/// The whole-file checksum is additive over the 4-byte-aligned table layout, so -/// it equals `checksum(header+directory) + Σ checksum(table)` — computed -/// analytically and the adjustment patched into `head` before the bytes are -/// concatenated. +/// checksums and `head.checkSumAdjustment`. @p tables need not be sorted; a +/// `head` table is patched in place with the final adjustment. [[nodiscard]] std::string build_sfnt(std::uint32_t sfnt_version, std::vector> tables); -/// Serialize a code point -> glyph map into a `cmap` table. -/// -/// LIMITATION: emits a single Windows (3,1) format-4 subtable, so only BMP code -/// points (<= U+FFFF) are supported; a map containing a code point beyond the -/// BMP (which would require a format-12 subtable) throws `std::runtime_error`. -/// Within the BMP there is no further restriction: the map is split into -/// maximal arithmetic runs (consecutive code points mapping to consecutive -/// glyphs, `idRangeOffset = 0`), and a run of length one is trivially -/// arithmetic, so the `glyphIdArray` path is never needed. This covers the -/// uniform PUA re-encode (one run) and ordinary remaps; format-12 / multi-plane -/// coverage is a follow-up. +/// Serialize a code point -> glyph map into a `cmap` table: one Windows +/// subtable, format 4 (3,1) while the map stays in the BMP, format 12 (3,10) +/// once it reaches beyond it. Both split the map into maximal runs of +/// consecutive code points mapping to consecutive glyphs, so format 4 never +/// needs a `glyphIdArray`. [[nodiscard]] std::string serialize_cmap(const std::map &map); -/// Serialize a minimal `name` table: nameIDs 1/2/4/6 (family / subfamily / -/// full / PostScript), Windows platform (3,1), UTF-16BE. -/// -/// OTS (the font sanitizer in Chrome/Firefox) requires `name` and rejects the -/// whole font when it is absent — like `post` and `OS/2`, PDF-embedded fonts -/// routinely omit it (a bare CFF has none; TrueType subsets often drop it). -/// An empty @p font_name falls back to "ODR Font". +// OTS (the font sanitizer in Chrome/Firefox) rejects a font that is missing +// `name`, `post` or `OS/2` — the browser then drops the `@font-face` and +// renders tofu — and PDF-embedded fonts routinely omit all three. The three +// below synthesize minimal stand-ins; OTS reconciles the fields they leave +// neutral (e.g. `fsSelection` against `head`). + +/// nameIDs 1/2/4/6 (family / subfamily / full / PostScript), Windows (3,1), +/// UTF-16BE. An empty @p font_name falls back to "ODR Font". [[nodiscard]] std::string serialize_name(const std::string &font_name); -/// Serialize a minimal version-3.0 `post` table (header only, no glyph names). -/// -/// OTS (the font sanitizer in Chrome/Firefox) lists `post` among the tables an -/// SFNT must carry and rejects the whole font when it is absent — the browser -/// then drops the `@font-face` and renders tofu. PDF-embedded TrueType fonts -/// routinely omit `post` (the viewer needs no glyph names), so a font copied -/// through verbatim would be rejected. Format 3.0 declares "no glyph names", -/// which is all a re-encoded display font needs. +/// Version-3.0 `post`: the header alone, i.e. "no glyph names". [[nodiscard]] std::string serialize_post(); -/// Serialize a minimal version-4 `OS/2` table. -/// -/// OTS (the font sanitizer in Chrome/Firefox) also requires `OS/2` and rejects -/// the whole font when it is absent — like `post`, PDF-embedded TrueType fonts -/// routinely omit it (the viewer takes its metrics elsewhere), so a font copied -/// through verbatim is rejected. The synthesized table carries neutral weight -/// and width (regular, medium) with the vertical metrics and character-range -/// bounds derived from the arguments; OTS reconciles the remaining fields (e.g. -/// the `fsSelection` style bits against `head`). @p units_per_em scales the +/// Version-4 `OS/2`, neutral weight and width. @p units_per_em scales the /// sub/superscript and strikeout defaults; @p y_min / @p y_max are the font /// bounding box (ascender/descender fall back to 0.8/0.2 em when degenerate); /// @p first_char / @p last_char bound the `cmap`. @@ -88,15 +72,10 @@ serialize_cmap(const std::map &map); /// never reached — when loaded via `@font-face`. `font.write()` then emits the /// re-encoded SFNT. /// -/// @p extra adds real-Unicode -> glyph entries alongside the PUA range, so a -/// run whose codes map 1:1 to those scalars can render the *real* Unicode -/// directly (the HTML layer then collapses its dual selectable/visible spans -/// into one). Keys must be in the BMP and outside the PUA (`U+E000..U+F8FF`) so -/// they never shadow a glyph's own PUA code point; the caller guarantees this. -/// The PUA range is always kept as a fallback. -/// -/// Throws `std::runtime_error` if the glyph count exceeds the BMP PUA capacity -/// (6400); multi-plane PUA spill-over is a follow-up. +/// @p extra adds real-Unicode -> glyph entries alongside the PUA range (see +/// `pua_cmap`), so a run whose codes map 1:1 to those scalars can render the +/// *real* Unicode directly — the HTML layer then collapses its dual +/// selectable/visible spans into one. void reencode_to_pua(sfnt::SfntFont &font, const std::map &extra = {}); diff --git a/src/odr/internal/font/type1_charstring.cpp b/src/odr/internal/font/type1_charstring.cpp index 0ccdccb75..12f084b17 100644 --- a/src/odr/internal/font/type1_charstring.cpp +++ b/src/odr/internal/font/type1_charstring.cpp @@ -2,14 +2,27 @@ #include #include +#include #include +#include #include +#include #include namespace odr::internal::font::type1 { namespace { +/// Charstring byte at @p i; an operand cut short by the end of the charstring +/// is malformed input, not something to read past. +[[nodiscard]] std::uint8_t byte_at(const std::string_view cs, + const std::size_t i) { + if (i >= cs.size()) { + throw std::runtime_error("type1: truncated charstring"); + } + return static_cast(cs[i]); +} + // Type1 charstring operators (single byte; 12 = escape to a two-byte op). enum T1 : std::int32_t { t1_hstem = 1, @@ -127,18 +140,18 @@ class Translator { p += 1; } else if (b <= 250) { value = (static_cast(b) - 247) * 256 + - static_cast(cs[p + 1]) + 108; + byte_at(cs, p + 1) + 108; p += 2; } else if (b <= 254) { value = -(static_cast(b) - 251) * 256 - - static_cast(cs[p + 1]) - 108; + byte_at(cs, p + 1) - 108; p += 2; } else { // 255: Type1 32-bit integer value = static_cast( - (static_cast(cs[p + 1]) << 24) | - (static_cast(cs[p + 2]) << 16) | - (static_cast(cs[p + 3]) << 8) | - static_cast(cs[p + 4])); + static_cast(byte_at(cs, p + 1)) << 24 | + static_cast(byte_at(cs, p + 2)) << 16 | + static_cast(byte_at(cs, p + 3)) << 8 | + byte_at(cs, p + 4)); p += 5; } m_stack.push_back(value); @@ -147,7 +160,7 @@ class Translator { std::int32_t op = b; ++p; if (b == 12) { - op = 1200 + static_cast(cs[p]); + op = 1200 + byte_at(cs, p); ++p; } handle(op, depth); diff --git a/src/odr/internal/font/type1_charstring.hpp b/src/odr/internal/font/type1_charstring.hpp index 86c333ac4..700881dd3 100644 --- a/src/odr/internal/font/type1_charstring.hpp +++ b/src/odr/internal/font/type1_charstring.hpp @@ -24,7 +24,8 @@ struct Type2Charstring { /// charstring, so the caller emits it against the CFF `nominalWidthX`. /// /// Best-effort and display-oriented: hints are dropped (they affect rendering -/// quality, not glyph shape), and unknown operators are skipped. +/// quality, not glyph shape), and unknown operators are skipped. Throws +/// `std::runtime_error` on a charstring that ends mid-operand. [[nodiscard]] Type2Charstring to_type2(std::string_view type1, const std::vector &subrs); diff --git a/src/odr/internal/font/type1_font.cpp b/src/odr/internal/font/type1_font.cpp index 2e4075b0b..8b02b6320 100644 --- a/src/odr/internal/font/type1_font.cpp +++ b/src/odr/internal/font/type1_font.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -56,6 +57,12 @@ namespace { } } +/// A `/FontBBox` number as an FWord. Clamping keeps an out-of-range number out +/// of the undefined double -> int16 conversion. +[[nodiscard]] std::int16_t to_fword(const double value) { + return static_cast(std::clamp(value, -32768.0, 32767.0)); +} + /// Parse the numbers inside the next `[...]` or `{...}` after @p key in @p s. [[nodiscard]] std::vector parse_number_array(const std::string_view s, const std::string_view key) { @@ -180,9 +187,8 @@ void Type1Font::parse_clear(const std::string_view clear) { } if (const std::vector bbox = parse_number_array(clear, "/FontBBox"); bbox.size() == 4) { - m_font_bbox = { - static_cast(bbox[0]), static_cast(bbox[1]), - static_cast(bbox[2]), static_cast(bbox[3])}; + m_font_bbox = {to_fword(bbox[0]), to_fword(bbox[1]), to_fword(bbox[2]), + to_fword(bbox[3])}; } // /Encoding: `StandardEncoding def`, or a custom array built with @@ -212,30 +218,36 @@ void Type1Font::parse_clear(const std::string_view clear) { } void Type1Font::parse_private(const std::string_view decrypted) { - std::int32_t len_iv = 4; + std::size_t len_iv = 4; if (const std::size_t k = decrypted.find("/lenIV"); k != std::string_view::npos) { std::size_t p = k + 6; std::int32_t value = 0; if (parse_int(read_token(decrypted, p), value)) { - len_iv = value; + if (value < 0) { + throw std::runtime_error("type1: negative /lenIV"); + } + len_iv = static_cast(value); } } - m_len_iv = len_iv; + + // /CharStrings starts where /Subrs ends (Subrs precede it). + const std::size_t cs = decrypted.find("/CharStrings"); // /Subrs: entries `dup RD NP`. if (const std::size_t k = decrypted.find("/Subrs"); k != std::string_view::npos) { std::size_t p = k; while ((p = decrypted.find("dup ", p)) != std::string_view::npos) { - // Stop when /CharStrings starts (Subrs precede it). - const std::size_t cs = decrypted.find("/CharStrings"); if (cs != std::string_view::npos && p > cs) { break; } std::size_t q = p + 4; std::int32_t index = 0; - if (!parse_int(read_token(decrypted, q), index) || index < 0) { + // Every subr needs at least one byte of input, so an index at or past the + // input size cannot name one — and must not size the vector. + if (!parse_int(read_token(decrypted, q), index) || index < 0 || + static_cast(index) >= decrypted.size()) { p += 4; continue; } @@ -254,7 +266,6 @@ void Type1Font::parse_private(const std::string_view decrypted) { } // /CharStrings: entries `/ RD ND`. - const std::size_t cs = decrypted.find("/CharStrings"); if (cs == std::string_view::npos) { return; } diff --git a/src/odr/internal/font/type1_font.hpp b/src/odr/internal/font/type1_font.hpp index 68f3a492b..6abac2de0 100644 --- a/src/odr/internal/font/type1_font.hpp +++ b/src/odr/internal/font/type1_font.hpp @@ -22,12 +22,10 @@ struct Glyph { /// /// A Type1 program has three sections: a clear-text header (font dictionary up /// to `eexec`), an `eexec`-encrypted private portion (`/Subrs`, -/// `/CharStrings`), and a zero-padded trailer. This reads the header for -/// `/FontMatrix`, -/// `/FontBBox`, `/Encoding` and `/FontName`, decrypts the `eexec` section -/// (`type1_crypt`) and extracts every glyph's decrypted charstring plus the -/// `/Subrs`. It does **not** yet interpret the charstrings — that is the -/// Type1 -> Type2 translation that follows, feeding 3.4's CFF -> OTF path. +/// `/CharStrings`), and a zero-padded trailer. This reads `/FontMatrix`, +/// `/FontBBox`, `/Encoding` and `/FontName` from the header, decrypts the +/// `eexec` section (`type1_crypt`) and keeps every glyph's decrypted +/// charstring plus the `/Subrs`; interpreting them is `type1_charstring`. /// /// Throws `std::runtime_error` when the program has no `eexec` section or no /// `/CharStrings`. @@ -78,7 +76,6 @@ class Type1Font { bool m_standard_encoding{true}; std::vector m_glyphs; std::vector m_subrs; - std::int32_t m_len_iv{4}; }; } // namespace odr::internal::font::type1 diff --git a/src/odr/internal/font/type1_transform.hpp b/src/odr/internal/font/type1_transform.hpp index 4a45a81a5..fbef4a989 100644 --- a/src/odr/internal/font/type1_transform.hpp +++ b/src/odr/internal/font/type1_transform.hpp @@ -10,11 +10,9 @@ class Type1Font; /// glyph's charstring to Type2 (`to_type2`, flattening the font's `/Subrs`) and /// assemble via the CFF builder, with `.notdef` placed at glyph 0. /// -/// Returns the CFF bytes (not a `cff::CffFont`): a `CffFont` is the -/// parse-and-keep-the-bytes reader, so producing one means parsing this output -/// back — the caller does that (`CffFont{to_cff(font)}`), then `wrap_to_otf` -/// wraps it for the browser, so an embedded Type1 font reuses the entire 3.4 -/// CFF path. Mirrors `cff::wrap_to_otf`, which likewise emits bytes. +/// Returns the CFF bytes, not a `cff::CffFont`: the caller parses them back +/// (`CffFont{to_cff(font)}`) and hands the result to `wrap_to_otf`, so an +/// embedded Type1 font reuses the whole CFF path. [[nodiscard]] std::string to_cff(const Type1Font &font); } // namespace odr::internal::font::type1 diff --git a/src/odr/internal/html/document.cpp b/src/odr/internal/html/document.cpp index d9ff9df81..40554e6df 100644 --- a/src/odr/internal/html/document.cpp +++ b/src/odr/internal/html/document.cpp @@ -23,14 +23,19 @@ namespace odr::internal::html { namespace { +/// Whether the document renders as fixed-size pages on a backdrop rather than +/// reflowing to the viewport. +bool is_paged_content(const Document &document, const HtmlConfig &config) { + return (document.document_type() == DocumentType::text && + config.text_document_margin) || + document.document_type() == DocumentType::presentation || + document.document_type() == DocumentType::drawing; +} + void front(const Document &document, const WritingState &state) { HtmlWriter &out = state.out(); - const bool paged_content = - (document.document_type() == DocumentType::text && - state.config().text_document_margin) || - document.document_type() == DocumentType::presentation || - document.document_type() == DocumentType::drawing; + const bool paged_content = is_paged_content(document, state.config()); out.write_begin(); out.write_header_begin(); @@ -78,13 +83,7 @@ void front(const Document &document, const WritingState &state) { void back(const Document &document, const WritingState &state) { HtmlWriter &out = state.out(); - const bool paged_content = - (document.document_type() == DocumentType::text && - state.config().text_document_margin) || - document.document_type() == DocumentType::presentation || - document.document_type() == DocumentType::drawing; - - if (paged_content) { + if (is_paged_content(document, state.config())) { out.write_element_end("div"); } @@ -323,56 +322,30 @@ class TextHtmlFragment final : public HtmlFragmentBase { } }; -class SlideHtmlFragment final : public HtmlFragmentBase { +/// A fragment rendering one top-level element handle (slide, sheet, page) +/// through its `translate_*` function. +template +class ElementHtmlFragment final : public HtmlFragmentBase { public: - explicit SlideHtmlFragment(std::string name, const std::size_t index, - std::string path, Document document, - const Slide &slide) + explicit ElementHtmlFragment(std::string name, const std::size_t index, + std::string path, Document document, + const Handle &element) : HtmlFragmentBase(std::move(name), index, std::move(path), std::move(document)), - m_slide{slide} {} + m_element{element} {} void write_fragment(HtmlWriter &, WritingState &state) const override { - translate_slide(m_slide, state); + Translate(m_element, state); } private: - Slide m_slide; + Handle m_element; }; -class SheetHtmlFragment final : public HtmlFragmentBase { -public: - explicit SheetHtmlFragment(std::string name, const std::size_t index, - std::string path, Document document, - const Sheet &sheet) - : HtmlFragmentBase(std::move(name), index, std::move(path), - std::move(document)), - m_sheet{sheet} {} - - void write_fragment(HtmlWriter &, WritingState &state) const override { - translate_sheet(m_sheet, state); - } - -private: - Sheet m_sheet; -}; - -class PageHtmlFragment final : public HtmlFragmentBase { -public: - explicit PageHtmlFragment(std::string name, const std::size_t index, - std::string path, Document document, - const Page &page) - : HtmlFragmentBase(std::move(name), index, std::move(path), - std::move(document)), - m_page{page} {} - - void write_fragment(HtmlWriter &, WritingState &state) const override { - translate_page(m_page, state); - } - -private: - Page m_page; -}; +using SlideHtmlFragment = ElementHtmlFragment; +using SheetHtmlFragment = ElementHtmlFragment; +using PageHtmlFragment = ElementHtmlFragment; } // namespace } // namespace odr::internal::html diff --git a/src/odr/internal/html/document_element.cpp b/src/odr/internal/html/document_element.cpp index 3376f346e..1630a9b9b 100644 --- a/src/odr/internal/html/document_element.cpp +++ b/src/odr/internal/html/document_element.cpp @@ -23,40 +23,58 @@ void html::translate_children(const ElementRange &range, void html::translate_element(const Element &element, const WritingState &state) { - if (element.type() == ElementType::text) { + switch (element.type()) { + case ElementType::text: translate_text(element, state); - } else if (element.type() == ElementType::line_break) { + break; + case ElementType::line_break: translate_line_break(element, state); - } else if (element.type() == ElementType::paragraph) { + break; + case ElementType::paragraph: translate_paragraph(element, state); - } else if (element.type() == ElementType::span) { + break; + case ElementType::span: translate_span(element, state); - } else if (element.type() == ElementType::link) { + break; + case ElementType::link: translate_link(element, state); - } else if (element.type() == ElementType::bookmark) { + break; + case ElementType::bookmark: translate_bookmark(element, state); - } else if (element.type() == ElementType::list) { + break; + case ElementType::list: translate_list(element, state); - } else if (element.type() == ElementType::list_item) { + break; + case ElementType::list_item: translate_list_item(element, state); - } else if (element.type() == ElementType::table) { + break; + case ElementType::table: translate_table(element, state); - } else if (element.type() == ElementType::frame) { + break; + case ElementType::frame: translate_frame(element, state); - } else if (element.type() == ElementType::image) { + break; + case ElementType::image: translate_image(element, state); - } else if (element.type() == ElementType::rect) { + break; + case ElementType::rect: translate_rect(element, state); - } else if (element.type() == ElementType::line) { + break; + case ElementType::line: translate_line(element, state); - } else if (element.type() == ElementType::circle) { + break; + case ElementType::circle: translate_circle(element, state); - } else if (element.type() == ElementType::custom_shape) { + break; + case ElementType::custom_shape: translate_custom_shape(element, state); - } else if (element.type() == ElementType::group) { + break; + case ElementType::group: translate_children(element.children(), state); - } else { + break; + default: // TODO log + break; } } @@ -196,36 +214,34 @@ void html::translate_sheet(const Sheet &sheet, const WritingState &state) { state.out().write_element_end("table"); } -void html::translate_slide(const Slide &slide, const WritingState &state) { +namespace { + +/// A slide or a drawing page: the master page's content under the page's own, +/// inside one outer page box. There is no inner (margin) box — unlike a text +/// document, both anchor their children absolutely at page coordinates. +template +void translate_page_like(const PageLike &page, + const html::WritingState &state) { state.out().write_element_begin( - "div", HtmlElementOptions() - .set_class("odr-page-outer") - .set_style(translate_outer_page_style(slide.page_layout()))); - // state.out().write_element_begin( - // "div", HtmlElementOptions().set_class("odr-page-inner").set_style( - // translate_inner_page_style(slide.page_layout()))); + "div", + html::HtmlElementOptions() + .set_class("odr-page-outer") + .set_style(html::translate_outer_page_style(page.page_layout()))); - translate_master_page(slide.master_page(), state); - translate_children(slide.children(), state); + html::translate_master_page(page.master_page(), state); + html::translate_children(page.children(), state); - // state.out().write_element_end("div"); state.out().write_element_end("div"); } -void html::translate_page(const Page &page, const WritingState &state) { - state.out().write_element_begin( - "div", HtmlElementOptions() - .set_class("odr-page-outer") - .set_style(translate_outer_page_style(page.page_layout()))); - // state.out().write_element_begin( - // "div", HtmlElementOptions().set_class("odr-page-inner").set_style( - // translate_inner_page_style(page.page_layout()))); +} // namespace - translate_master_page(page.master_page(), state); - translate_children(page.children(), state); +void html::translate_slide(const Slide &slide, const WritingState &state) { + translate_page_like(slide, state); +} - // state.out().write_element_end("div"); - state.out().write_element_end("div"); +void html::translate_page(const Page &page, const WritingState &state) { + translate_page_like(page, state); } void html::translate_master_page(const MasterPage &masterPage, @@ -307,7 +323,7 @@ void html::translate_link(const Element &element, const WritingState &state) { state.out().write_element_begin( "a", HtmlElementOptions().set_inline(true).set_attributes( - HtmlAttributesVector{{"href", link.href()}})); + HtmlAttributesVector{{"href", escape_attribute(link.href())}})); translate_children(link.children(), state); state.out().write_element_end("a"); } @@ -317,8 +333,9 @@ void html::translate_bookmark(const Element &element, const Bookmark bookmark = element.as_bookmark(); state.out().write_element_begin( - "a", HtmlElementOptions().set_inline(true).set_attributes( - HtmlAttributesVector{{"id", bookmark.name()}})); + "a", + HtmlElementOptions().set_inline(true).set_attributes( + HtmlAttributesVector{{"id", escape_attribute(bookmark.name())}})); state.out().write_element_end("a"); } @@ -425,7 +442,7 @@ void html::translate_image(const Element &element, const WritingState &state) { .set_attributes([&](const HtmlAttributeWriterCallback &clb) { clb("alt", "Error: image not found or unsupported"); if (resource_location.has_value()) { - clb("src", resource_location.value()); + clb("src", escape_attribute(resource_location.value())); } else { clb("src", [&](std::ostream &o) { // reached only for internal images, which have a file diff --git a/src/odr/internal/html/document_style.cpp b/src/odr/internal/html/document_style.cpp index fb87a4218..4883ae084 100644 --- a/src/odr/internal/html/document_style.cpp +++ b/src/odr/internal/html/document_style.cpp @@ -142,7 +142,9 @@ std::string html::translate_text_style(const TextStyle &text_style) { std::string result; if (const std::optional font_name = text_style.font_name; font_name.has_value()) { - result.append("font-family:").append(*font_name).append(";"); + result.append("font-family:") + .append(escape_attribute(std::string(*font_name))) + .append(";"); } if (const std::optional font_size = text_style.font_size; font_size.has_value()) { @@ -168,7 +170,9 @@ std::string html::translate_text_style(const TextStyle &text_style) { } if (const std::optional font_shadow = text_style.font_shadow; font_shadow.has_value()) { - result.append("text-shadow:").append(*font_shadow).append(";"); + result.append("text-shadow:") + .append(escape_attribute(*font_shadow)) + .append(";"); } if (const std::optional font_color = text_style.font_color; font_color.has_value()) { @@ -329,21 +333,29 @@ html::translate_table_cell_style(const TableCellStyle &table_cell_style) { if (const std::optional border_right = table_cell_style.border.right; border_right.has_value()) { - result.append("border-right:").append(*border_right).append(";"); + result.append("border-right:") + .append(escape_attribute(*border_right)) + .append(";"); } if (const std::optional border_top = table_cell_style.border.top; border_top.has_value()) { - result.append("border-top:").append(*border_top).append(";"); + result.append("border-top:") + .append(escape_attribute(*border_top)) + .append(";"); } if (const std::optional border_left = table_cell_style.border.left; border_left.has_value()) { - result.append("border-left:").append(*border_left).append(";"); + result.append("border-left:") + .append(escape_attribute(*border_left)) + .append(";"); } if (const std::optional border_bottom = table_cell_style.border.bottom; border_bottom.has_value()) { - result.append("border-bottom:").append(*border_bottom).append(";"); + result.append("border-bottom:") + .append(escape_attribute(*border_bottom)) + .append(";"); } if (const std::optional text_rotation = table_cell_style.text_rotation; diff --git a/src/odr/internal/html/filesystem.cpp b/src/odr/internal/html/filesystem.cpp index 24f75447e..ea2bb719d 100644 --- a/src/odr/internal/html/filesystem.cpp +++ b/src/odr/internal/html/filesystem.cpp @@ -28,11 +28,7 @@ class HtmlServiceImpl final : public HtmlService { [[nodiscard]] const HtmlViews &list_views() const override { return m_views; } [[nodiscard]] bool exists(const std::string &path) const override { - if (path == "files.html") { - return true; - } - - return false; + return path == "files.html"; } [[nodiscard]] std::string mimetype(const std::string &path) const override { @@ -81,46 +77,38 @@ class HtmlServiceImpl final : public HtmlService { out.write_body_begin(); + const auto span = [&out](const HtmlWritable &content) { + out.write_element_begin("span"); + out.write_raw(content); + out.write_element_end("span"); + }; + for (; !file_walker.end(); file_walker.next()) { - Path file_path(file_walker.path()); + const Path file_path(file_walker.path()); const bool is_file = file_walker.is_file(); out.write_element_begin("p"); - out.write_element_begin("span"); - out.write_raw(file_path.string()); - out.write_element_end("span"); - - out.write_element_begin("span"); - out.write_raw(" "); - out.write_element_end("span"); - - out.write_element_begin("span"); - out.write_raw(file_walker.is_file() ? "file" : "directory"); - out.write_element_end("span"); + span(escape_text(file_path.string())); + span(" "); + span(is_file ? "file" : "directory"); if (is_file) { - out.write_element_begin("span"); - out.write_raw(" "); - out.write_element_end("span"); + span(" "); File file = m_filesystem.open(file_path.string()); - out.write_element_begin("span"); - out.write_raw(std::to_string(file.size())); - out.write_element_end("span"); + span(std::to_string(file.size())); if (const std::unique_ptr stream = file.stream(); stream != nullptr) { - out.write_element_begin("span"); - out.write_raw(" "); - out.write_element_end("span"); + span(" "); out.write_element_begin( "a", HtmlElementOptions().set_attributes(HtmlAttributesVector{ {"href", file_to_url(*stream, "application/octet-stream")}, - {"download", file_path.basename()}})); + {"download", escape_attribute(file_path.basename())}})); out.write_raw("download"); out.write_element_end("a"); } diff --git a/src/odr/internal/html/image_file.cpp b/src/odr/internal/html/image_file.cpp index 433a3ab65..7a8da9b34 100644 --- a/src/odr/internal/html/image_file.cpp +++ b/src/odr/internal/html/image_file.cpp @@ -65,11 +65,7 @@ class HtmlServiceImpl final : public HtmlService { [[nodiscard]] const HtmlViews &list_views() const override { return m_views; } [[nodiscard]] bool exists(const std::string &path) const override { - if (path == "image.html") { - return true; - } - - return false; + return path == "image.html"; } [[nodiscard]] std::string mimetype(const std::string &path) const override { diff --git a/src/odr/internal/html/pdf_file.cpp b/src/odr/internal/html/pdf_file.cpp index a2a5f062f..f099bb2d5 100644 --- a/src/odr/internal/html/pdf_file.cpp +++ b/src/odr/internal/html/pdf_file.cpp @@ -43,17 +43,15 @@ namespace odr::internal::html { namespace { -/// Round to 0.01 user-space units; sub-precision beyond that is invisible and -/// the extra digits add up across a page full of path data. +/// Round to 0.01 units; finer precision is invisible and the extra digits add +/// up across a page full of path data. double round2(const double v) { return std::round(v * 100.0) / 100.0; } constexpr double pt_to_in = 1.0 / 72.0; -/// Serialize a transform as an SVG `matrix(...)`. Only the translation (e, f) -/// is rounded — it lives in page-box units where 1/100 px is plenty; the linear -/// part (a..d) keeps full precision so small scale/skew factors aren't -/// quantized to zero. Used for `transform`, `gradientTransform` and -/// `patternTransform`. +/// A transform as an SVG `matrix(...)`. Only the translation is rounded; the +/// linear part keeps full precision so small scale/skew factors aren't +/// quantized to zero. std::string svg_matrix(const util::math::Transform2D &m) { std::ostringstream f; f << "matrix(" << m.a << ',' << m.b << ',' << m.c << ',' << m.d << ',' @@ -73,14 +71,12 @@ struct LinkOut { ///< `target="_self"`) }; -/// Maps a link's 0-based target page index to the href navigating to it: a -/// "#pN" anchor in the combined document, a page-view file name in a -/// standalone page. Returns "" to drop the link (target page not rendered). +/// A link's 0-based target page index to the href navigating to it. Returns "" +/// to drop the link (target page not rendered). using PageHref = std::function; -/// Resolves a link annotation's destination to a page: a `page-object -> -/// 0-based index` map plus the catalog's named-destination table (`/Dests` -/// dictionary and the `/Names /Dests` name tree, ISO 32000-1 12.3.2.3). +/// Resolves a link annotation's destination to a page index, via the catalog's +/// named-destination tables (ISO 32000-1 12.3.2.3). struct LinkResolver { pdf::DocumentParser &parser; std::map page_index; @@ -180,25 +176,22 @@ LinkResolver build_link_resolver(pdf::DocumentParser &parser, return resolver; } -/// Whether a `/URI` action target is safe to emit as an `href`. A PDF is -/// untrusted input, so active schemes (`javascript:`, `data:`, `vbscript:`, …) -/// must not become a clickable link that executes in the generated document. -/// We allow only the common navigable schemes plus scheme-less (relative) -/// references. Embedded ASCII whitespace/control bytes are ignored when reading -/// the scheme, matching browsers that strip them before dispatch (so -/// `java\tscript:` cannot slip through). +/// Whether a `/URI` action target is safe to emit as an `href`: only the +/// navigable schemes plus scheme-less (relative) references, so `javascript:` +/// and friends cannot become a clickable link. Embedded whitespace/control +/// bytes are ignored while reading the scheme, as browsers strip them before +/// dispatch (`java\tscript:` must not slip through). bool is_safe_uri(std::string_view uri) { std::string scheme; for (const char ch : uri) { const auto c = static_cast(ch); if (ch == ':') { - for (char &s : scheme) { - s = static_cast(std::tolower(static_cast(s))); - } - static constexpr std::string_view allowed[] = {"http", "https", "mailto", - "ftp", "ftps", "tel"}; - return std::find(std::begin(allowed), std::end(allowed), scheme) != - std::end(allowed); + std::ranges::transform(scheme, scheme.begin(), [](const char s) { + return static_cast(std::tolower(static_cast(s))); + }); + static constexpr std::array allowed = { + "http", "https", "mailto", "ftp", "ftps", "tel"}; + return std::ranges::find(allowed, scheme) != allowed.end(); } if (ch == '/' || ch == '?' || ch == '#') { return true; // path/query/fragment reached first -> relative reference @@ -215,10 +208,8 @@ bool is_safe_uri(std::string_view uri) { return true; // no ':' -> relative reference } -/// Resolve a page's `/Link` annotations (ISO 32000-1 12.5.6.5) to positioned -/// overlays: a `/URI` action becomes an external link, a `/GoTo` action or a -/// direct `/Dest` an internal link via `page_href`. `to_box` maps PDF user -/// space to the page box (points, y-down). +/// A page's `/Link` annotations (ISO 32000-1 12.5.6.5) as positioned overlays. +/// `to_box` maps PDF user space to the page box (points, y-down). std::vector collect_page_links(const pdf::Page &page, const util::math::Transform2D &to_box, LinkResolver &resolver, @@ -294,8 +285,7 @@ void write_page_links(HtmlWriter &out, const std::vector &links) { for (const LinkOut &link : links) { std::ostringstream a; // Internal `#pN` links must override the document's `` so they scroll within the rendered PDF instead of - // opening a new copy. + // target="_blank">` or they open a new copy instead of scrolling. a << " &rgb) { return std::move(s).str(); } -/// Map a PDF blend-mode name (`/ExtGState` `/BM`, ISO 32000-1 11.3.5) to its -/// CSS `mix-blend-mode` keyword. CSS derives its blend modes from PDF, so the -/// separable and non-separable modes map 1:1 (camelCase -> kebab-case). Returns -/// "" for `Normal` and for any unrecognized name (rendered normal), so a caller -/// can skip the property entirely. +/// A PDF blend-mode name (`/BM`, ISO 32000-1 11.3.5) as its CSS +/// `mix-blend-mode` keyword — CSS took its blend modes from PDF, so they map +/// 1:1. "" for `Normal` and for anything unrecognized, so callers can skip the +/// property entirely. std::string blend_mode_to_css(const std::string &blend_mode) { static const std::unordered_map map = { {"Multiply", "multiply"}, {"Screen", "screen"}, @@ -376,9 +363,8 @@ std::string blend_mode_to_css(const std::string &blend_mode) { } /// The CSS declaration a non-embedded font renders through: its substitute -/// `font-family` stack plus the weight/style implied by the `/BaseFont` name -/// and `/FontDescriptor` flags. Interned as an `ff` atomic class on the -/// fallback (`font == 0`) runs of either text mode. +/// family stack plus the weight/style implied by `/BaseFont` and the +/// `/FontDescriptor` flags. std::string font_substitute_declaration(const pdf::FontSubstitute &substitute) { std::string declaration = "font-family:" + substitute.css_family; if (substitute.bold) { @@ -390,10 +376,8 @@ std::string font_substitute_declaration(const pdf::FontSubstitute &substitute) { return declaration; } -/// The `local(...)` sources of a CSS `font-family` stack, dropping the generic -/// keywords an `@font-face src` cannot name. Returns e.g. -/// "local('Times New Roman'),local(Times)" for "'Times New Roman',Times,serif", -/// or "" when the stack names no concrete font (generic-only). +/// The `local(...)` sources of a `font-family` stack, dropping the generic +/// keywords an `@font-face src` cannot name. "" when the stack is generic-only. std::string local_font_sources(const std::string_view css_family) { static constexpr std::array generics = { "serif", "sans-serif", "monospace", "cursive", "fantasy", "system-ui"}; @@ -410,8 +394,7 @@ std::string local_font_sources(const std::string_view css_family) { while (!name.empty() && name.back() == ' ') { name.remove_suffix(1); } - const bool generic = - std::find(generics.begin(), generics.end(), name) != generics.end(); + const bool generic = std::ranges::find(generics, name) != generics.end(); if (!name.empty() && !generic) { if (!src.empty()) { src += ','; @@ -428,13 +411,10 @@ std::string local_font_sources(const std::string_view css_family) { return src; } -/// Registers one `@font-face` per (substitute family, style, ascent) that -/// overrides the face's ascent/descent so a glyph's baseline lands exactly at -/// the `top` `add_position_classes` derives from `ascent_em` — independent of -/// the metrics of whichever local font actually resolves. Without the override -/// the browser positions the baseline using the resolved font's own ascent, -/// which for a large non-embedded run (e.g. a 120pt Times title) drops it well -/// below the intended baseline. +/// One `@font-face` per (substitute family, style, ascent) overriding the +/// face's ascent/descent, so a glyph's baseline lands at the `top` +/// `add_position_classes` derived from `ascent_em` rather than wherever +/// whichever local font resolves would put it. class SubstituteFontFaces { public: /// The `font-family:...` (plus weight/style) declaration for `substitute`, @@ -446,10 +426,9 @@ class SubstituteFontFaces { if (src.empty()) { return font_substitute_declaration(substitute); } - // ascent-override + descent-override sum to one em, so `line-height:1` - // leaves no leading and the baseline sits at exactly `ascent_em` of the em - // box. `ascent_em` is clamped to [0.5, 1.2]; the `max` keeps descent - // non-negative for the rare ascent > 1 (a slight baseline approximation). + // The two overrides sum to one em, so `line-height:1` leaves no leading and + // the baseline sits at exactly `ascent_em`. `max` keeps descent + // non-negative for the rare clamped ascent > 1. const double ascent = ascent_em; const double descent = std::max(0.0, 1.0 - ascent_em); std::ostringstream key; @@ -489,9 +468,8 @@ class SubstituteFontFaces { std::vector m_faces; }; -/// Build an SVG `d` attribute from a path's subpaths, each point mapped through -/// `to_box` (PDF user space -> the page box, y-down). Lines become `L`, cubic -/// Béziers `C`, and an explicitly closed subpath ends with `Z`. +/// An SVG `d` attribute for a path's subpaths, each point mapped through +/// `to_box` (PDF user space -> the page box, y-down). std::string svg_path_d(const std::vector &subpaths, const util::math::Transform2D &to_box) { std::ostringstream d; @@ -522,13 +500,9 @@ std::string svg_path_d(const std::vector &subpaths, return std::move(d).str(); } -/// Serialize a painted path to an SVG `` fragment in the page -/// viewBox, or "" when it paints nothing. Fill honours the even-odd rule; -/// stroke carries width (CTM-scaled in user space), caps, joins, miter limit -/// and the dash pattern. A zero stroke width renders as a thin hairline. -/// `clip_id`, when non-empty, references a `` installed via -/// `clip-path`. `fill_url_id`, when non-empty, fills the path with that paint -/// server (a shading gradient or a tiling ``) instead of `fill_color`. +/// A painted path as an SVG `` fragment in the page viewBox, or "" +/// when it paints nothing. `clip_id` and `fill_url_id`, when non-empty, name a +/// `` and a paint server (gradient or tiling pattern) to reference. std::string svg_path_fragment(const pdf::PathElement &path, const util::math::Transform2D &to_box, const std::string &clip_id, @@ -563,8 +537,7 @@ std::string svg_path_fragment(const pdf::PathElement &path, if (path.stroke_alpha < 1) { f << " stroke-opacity=\"" << round2(path.stroke_alpha) << '"'; } - // A 0 width is "device-thinnest" in PDF; SVG would draw nothing, so floor - // it to a sub-point hairline. + // A 0 width is "device-thinnest" in PDF; SVG would draw nothing. const double width = path.line_width > 0 ? path.line_width : 0.5; f << " stroke-width=\"" << round2(width) << '"'; if (path.line_cap == 1) { @@ -603,16 +576,12 @@ std::string svg_path_fragment(const pdf::PathElement &path, return std::move(f).str(); } -/// Serialize an image XObject to an SVG `` fragment in the page viewBox, -/// or "" when it carries no pass-through bytes. The image fills the unit square -/// in user space (ISO 32000-1 8.10.5); the transform maps that square — through -/// a vertical flip (the image's first row is its top, SVG draws y-down) and the -/// CTM — into the page box. `clip_id`, when non-empty, installs a clip via a -/// wrapping ``. The clip geometry is in the page viewBox -/// (`userSpaceOnUse`), but the `` carries its own `transform`, so a -/// `clip-path` placed *on the image* would be resolved in the image's -/// post-transform unit-square space and clip the whole image away. The `` -/// carries no transform, so the clip is read in the viewBox where it lives. +/// An image XObject as an SVG `` fragment in the page viewBox, or "" +/// when it carries no pass-through bytes. The image fills the unit square in +/// user space (ISO 32000-1 8.10.5), flipped vertically because its first row is +/// its top. `clip_id` is installed on a wrapping ``, not on the ``: +/// the clip geometry is `userSpaceOnUse` in the viewBox, and on the image it +/// would resolve in the image's post-transform unit-square space instead. std::string svg_image_fragment(const pdf::ImageElement &image, const util::math::Transform2D &to_box, const std::string &clip_id) { @@ -643,11 +612,9 @@ std::string svg_image_fragment(const pdf::ImageElement &image, return std::move(f).str(); } -/// Shared bookkeeping for the per-page `` registries below (clips, -/// gradients, tiling patterns): a signature->id cache that deduplicates -/// repeated definitions, a per-page monotonic id counter, and the accumulated -/// `` markup (emitted once into the page's hidden ``). Ids are -/// namespaced per page as `_`. +/// Shared bookkeeping for the per-page `` registries below: a +/// signature->id cache deduplicating repeated definitions plus the accumulated +/// `` markup. Ids are namespaced per page as `_`. class DefsRegistry { public: explicit DefsRegistry(const std::uint32_t page) : m_page{page} {} @@ -655,9 +622,8 @@ class DefsRegistry { [[nodiscard]] std::string defs() const { return m_defs.str(); } protected: - /// The id for `signature`, minting `_` the first time it is - /// seen. `inserted` is true only on that first sight — when the caller still - /// needs to emit the definition into `m_defs`. + /// The id for `signature`. `inserted` is true only on first sight, when the + /// caller still has to emit the definition into `m_defs`. struct Entry { std::string id; bool inserted; @@ -679,12 +645,10 @@ class DefsRegistry { std::unordered_map m_id_by_signature; }; -/// Registers a page's clip regions as nested `` defs, deduplicating -/// shared prefixes. PDF's current clip is the *intersection* of an ordered list -/// of regions; SVG expresses intersection by chaining `clip-path` from one -/// `` to the next, so region i's clipPath references region i-1's and -/// the painted element references the last. Ids are namespaced per page -/// (`c_`). +/// A page's clip regions as nested `` defs (`c_`). PDF's +/// current clip is the *intersection* of an ordered region list; SVG expresses +/// intersection by chaining `clip-path`, so region i references region i-1 and +/// the painted element references the last. class ClipRegistry : public DefsRegistry { public: using DefsRegistry::DefsRegistry; @@ -718,18 +682,13 @@ class ClipRegistry : public DefsRegistry { } }; -/// Registers a page's shadings (axial/radial) as ``/ -/// `` defs, deduplicating by shading and placement. The -/// shading's pre-sampled colour stops become ``s; `gradientTransform` -/// (shading space -> page box) places the gradient in the page's user space, so -/// referencing elements use `gradientUnits="userSpaceOnUse"`. Ids are -/// namespaced per page (`g_`). +/// A page's axial/radial shadings as ``/`` defs +/// (`g_`), placed by `gradientTransform` in `userSpaceOnUse`. /// -/// DEFERRED (out of scope for this stage): PDF `/Extend` is approximated by -/// SVG's default `pad` spread (the end stops extend outward), so a non-extended -/// shading is over-painted beyond its interval instead of being masked to it; -/// `Shading::background` and `Shading::bbox` are likewise not yet honoured. -/// Honouring them needs the fill clipped to the gradient band/annulus. +/// DEFERRED: `/Extend` is approximated by SVG's default `pad` spread, so a +/// non-extended shading over-paints beyond its interval; `Shading::background` +/// and `Shading::bbox` are not honoured. Both need the fill clipped to the +/// gradient band/annulus. class GradientRegistry : public DefsRegistry { public: using DefsRegistry::DefsRegistry; @@ -773,10 +732,9 @@ class GradientRegistry : public DefsRegistry { } }; -/// Serialize an `sh` shading flood to an SVG `` covering the page box, -/// filled with `gradient_id` and bounded by `clip_id` (the clip in force at -/// `sh` time). Returns "" when the shading produced no gradient. The rect spans -/// the whole page; the clip (and the gradient's own extent) bound the paint. +/// An `sh` shading flood as an SVG `` spanning the page box, filled with +/// `gradient_id`; `clip_id` (and the gradient's own extent) bound the paint. +/// "" when the shading produced no gradient. std::string svg_shading_fragment(const std::string &gradient_id, const std::string &clip_id, const double width, const double height, const double alpha, @@ -800,17 +758,13 @@ std::string svg_shading_fragment(const std::string &gradient_id, return std::move(f).str(); } -/// Registers a page's tiling patterns (`/PatternType 1`) as SVG `` -/// defs. The pattern's content stream is run as a mini page (`extract_page`) -/// into tile fragments laid out in pattern space; the `` repeats them -/// every `/XStep`/`/YStep`, and `patternTransform` (pattern space -> page box) -/// places the lattice. An uncoloured pattern (`/PaintType 2`) ignores its -/// content's own colours and paints in the path's fill colour, so the cache key -/// folds that colour in. Each cell is clipped to its `/BBox` so marks outside -/// the cell (or in the gap when a step exceeds the BBox) don't leak into the -/// tile. Ids are namespaced per page (`pat_`). Only paths and images -/// inside the tile are rendered (nested text/shadings/patterns are skipped — -/// rare). Returns "" for an unrepresentable pattern. +/// A page's tiling patterns (`/PatternType 1`) as SVG `` defs +/// (`pat_`). The content stream is run as a mini page into tile +/// fragments in pattern space, repeated every `/XStep`/`/YStep` and placed by +/// `patternTransform`. An uncoloured pattern (`/PaintType 2`) paints in the +/// path's fill colour, so the cache key folds that colour in. Only paths and +/// images are rendered (nested text/shadings/patterns are skipped — rare). +/// "" for an unrepresentable pattern. class PatternRegistry : public DefsRegistry { public: using DefsRegistry::DefsRegistry; @@ -859,8 +813,7 @@ class PatternRegistry : public DefsRegistry { << "\" height=\"" << round2(std::abs(pattern.y_step)) << "\" patternTransform=\"" << svg_matrix(m) << "\">"; // Clip each cell to its `/BBox` (ISO 32000-1 8.7.3.1). An overlapping - // lattice (a step smaller than the BBox) can't be expressed as a single SVG - // `` and is not reproduced. + // lattice (step < BBox) has no single-`` equivalent and is lost. const double bbox_w = pattern.bbox[2] - pattern.bbox[0]; const double bbox_h = pattern.bbox[3] - pattern.bbox[1]; if (bbox_w > 0 && bbox_h > 0) { @@ -879,13 +832,11 @@ class PatternRegistry : public DefsRegistry { class MaskRegistry; -/// Serialize one graphic page element (a painted path, a shading flood, an -/// image, or a nested transparency group) to an SVG fragment in the page -/// viewBox, registering any clip, gradient, pattern or soft mask it needs. -/// Returns "" for a text element or one that paints nothing. A soft mask on the -/// element wraps its fragment in a masked ``; a `GroupElement` renders its -/// children then wraps them in one `` carrying the group's opacity, blend -/// and mask (so the group composites as a unit before those apply). +/// One graphic page element as an SVG fragment in the page viewBox, +/// registering any clip, gradient, pattern or soft mask it needs. "" for a text +/// element or one that paints nothing. A `GroupElement`'s children are wrapped +/// in a single `` so the group composites as a unit before its +/// opacity/blend/mask apply. std::string render_graphic_fragment(const pdf::PageElement &element, const util::math::Transform2D &to_box, double width, double height, @@ -894,14 +845,10 @@ std::string render_graphic_fragment(const pdf::PageElement &element, PatternRegistry &patterns, MaskRegistry &masks, const Logger &logger); -/// Registers a page's soft masks (`/SMask`, ISO 32000-1 11.6.5.2) as `` -/// defs. The extractor has rendered each mask's transparency group into a list -/// of graphic elements (in user space); those are serialized into the mask body -/// with the page's own clip/gradient/pattern registries, so their ids stay -/// unique within the page. Coverage comes from luminance by default -/// (`/Luminosity` -> the SVG mask default) or from alpha (`/Alpha` -> -/// `mask-type="alpha"`); a non-black `/BC` backdrop floods behind the group. -/// Ids are namespaced per page (`m_`). +/// A page's soft masks (`/SMask`, ISO 32000-1 11.6.5.2) as `` defs +/// (`m_`). The extractor has already rendered each mask's transparency +/// group to graphic elements; those are serialized through the page's own +/// clip/gradient/pattern registries so their ids stay unique within the page. class MaskRegistry : public DefsRegistry { public: using DefsRegistry::DefsRegistry; @@ -911,9 +858,8 @@ class MaskRegistry : public DefsRegistry { const double width, const double height, ClipRegistry &clips, GradientRegistry &gradients, PatternRegistry &patterns, const Logger &logger) { - // A fresh `SoftMask` is built for every `gs`, but many are identical (the - // same drop-shadow reused across a run of glyphs, say). Dedupe on the - // rendered body + type + backdrop so those collapse to a single def. + // A fresh `SoftMask` is built for every `gs`, but many are identical (one + // drop-shadow across a run of glyphs); dedupe on the rendered body. std::ostringstream body; for (const pdf::PageElement &element : mask.group) { body << render_graphic_fragment(element, to_box, width, height, clips, @@ -936,9 +882,7 @@ class MaskRegistry : public DefsRegistry { m_defs << " mask-type=\"alpha\""; } m_defs << '>'; - // A non-black `/BC` backdrop floods the mask region behind the group; the - // default (black) needs none — SVG's mask background is already luminance - // 0. + // Black — SVG's mask background already — needs no backdrop rect. if (mask.backdrop.has_value() && ((*mask.backdrop)[0] + (*mask.backdrop)[1] + (*mask.backdrop)[2] > 0)) { m_defs << "`, but text is painted as positioned markup, not SVG, so it cannot ride -/// inside that ``. The extractor faithfully nests such text in the group; to -/// avoid dropping it from both the visual and selection layers, each interior -/// `TextElement` is hoisted to the top level — where the ordinary text pipeline -/// renders it and `extract_text`-style top-level scans pick it up. The only -/// thing forgone is the group effect on the text itself (see pdf/AGENTS.md -/// gaps); the group's graphics are untouched and still composited as a unit. -/// Nesting is flattened recursively; hoisted text is emitted at the group's -/// position (ahead of the composited graphics), and a group left with no -/// graphics is dropped. +/// Hoists text out of transparency groups (recursively) to the top level. A +/// group's effects ride an SVG ``, but text is positioned markup, not SVG, +/// so it cannot sit inside that `` — without the hoist it would be dropped +/// from both the visual and the selection layer. Forgone: the group effect on +/// the text itself (see pdf/AGENTS.md gaps). The group's graphics are untouched +/// and still composite as a unit; a group left with none is dropped. std::vector lift_group_text(std::vector elements) { std::vector result; @@ -1097,14 +1035,11 @@ lift_group_text(std::vector elements) { return result; } -/// Deduplicates CSS declarations into atomic, single-property classes. PDF text -/// emits one absolutely-positioned line block per detected line, and the same -/// font sizes, offsets and spacings recur across the (potentially millions of) -/// elements. Writing each declaration inline bloats the document. Instead, -/// every distinct declaration is registered once here, named `` in -/// first-seen order (e.g. `f1`, `f2` for font sizes, `t1` for a top offset), -/// emitted once in , and referenced by class on each element. This is -/// representation-only: the computed style of every element is unchanged. +/// Deduplicates CSS declarations into atomic, single-property classes named +/// `` in first-seen order, emitted once in ``. The same font +/// sizes, offsets and spacings recur across up to millions of positioned +/// elements, and inline declarations bloat the document. Representation-only: +/// no element's computed style changes. class AtomicStyles { public: /// `prefix` selects the property family; `declaration` is a full CSS @@ -1121,9 +1056,8 @@ class AtomicStyles { return it->second; } - /// Writes one rule per line (`.f1{font-size:9.96pt}`) so regeneration diffs - /// stay legible. Each rule is preceded by a newline; the caller has already - /// written the constant rules on the opening `