From e02b7e2c79ea864c71b0a3b46f9f556c8b2f28db Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 26 Jul 2026 12:11:15 +0200 Subject: [PATCH 1/2] Serve the translated document over loopback instead of files odrcore rendered every page of a document before any of them was shown, on whichever thread called translate - the main one whenever a password was entered or editing was toggled. `odr::HttpServer`, which is what OpenDocument.droid already uses, turns that around: `translate` hands back a service that has not rendered anything, the service is connected to a server bound to 127.0.0.1:29665, and odrcore renders a page on one of the server's threads when the web view asks for it. So a page nobody opens is never rendered, the work that is done happens off the calling thread, and turning to another page is a navigation rather than another translation - `Document.page` no longer re-parses the file to show a page whose address it already had. The prefix in the URL changes on every translation. The web view caches by URL, and a document re-translated after a password or an edit has to come back at an address it has not seen before. Falling over now happens while the page is being served rather than in `translate`, so a 4xx/5xx from the server is routed back into `documentLoadingError` and the user sees the error page or the raw file as before, not the server's "Internal Server Error". `kCoreWrapperServesOverHttp` switches back to writing files, which is also what happens by itself when the port cannot be bound. It is there to keep that path working while the migration finishes, not as a runtime option. Permissions: none needed. A listening socket on loopback takes no entitlement, and the local network permission from iOS 14 covers the local subnet and multicast, not 127.0.0.1. App Transport Security does block plain HTTP, so both Info.plists gain NSAllowsLocalNetworking - the narrow exception for local addresses, which unlike NSAllowsArbitraryLoads needs no justification in App Store review. The conan profile grows a workaround: cpp-httplib resolves host names through CFHost on Apple platforms and odrcore's CMake links neither CoreFoundation nor CFNetwork, so its `cli/server` tool fails to link. The app links both frameworks itself. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011MhKU2kWm1cPW4GBq9gon5 --- OpenDocumentReader.xcodeproj/project.pbxproj | 8 + OpenDocumentReader/CoreWrapper.h | 6 +- OpenDocumentReader/CoreWrapper.mm | 151 +++++++++++++++++- OpenDocumentReader/Document.swift | 31 ++-- .../DocumentViewController.swift | 41 ++++- OpenDocumentReader/Info-Lite.plist | 7 + OpenDocumentReader/Info.plist | 7 + .../OpenDocumentReaderTests.swift | 115 +++++++++++-- README.md | 29 ++++ conan/conanfile.py | 2 +- conan/profiles/ios-base | 9 ++ 11 files changed, 369 insertions(+), 37 deletions(-) diff --git a/OpenDocumentReader.xcodeproj/project.pbxproj b/OpenDocumentReader.xcodeproj/project.pbxproj index 313e50e..19e1d86 100644 --- a/OpenDocumentReader.xcodeproj/project.pbxproj +++ b/OpenDocumentReader.xcodeproj/project.pbxproj @@ -36,6 +36,8 @@ E2A17B1000000000000000B0 /* test.ods in Resources */ = {isa = PBXBuildFile; fileRef = E2A17B1200000000000000B2 /* test.ods */; }; E2A17B1100000000000000B1 /* test.odp in Resources */ = {isa = PBXBuildFile; fileRef = E2A17B1300000000000000B3 /* test.odp */; }; E26C39392250DC6E009C484A /* WebKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = E26C39382250DC6E009C484A /* WebKit.framework */; }; + E2A17B2000000000000000C0 /* CoreFoundation.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = E2A17B2200000000000000C2 /* CoreFoundation.framework */; }; + E2A17B2100000000000000C1 /* CFNetwork.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = E2A17B2300000000000000C3 /* CFNetwork.framework */; }; E2C008FA220F1CF80097C594 /* CoreWrapper.mm in Sources */ = {isa = PBXBuildFile; fileRef = E2C008F9220F1CF80097C594 /* CoreWrapper.mm */; }; E2D0B3D9226D945400534FCC /* StoreReviewHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2D0B3D8226D945400534FCC /* StoreReviewHelper.swift */; }; E2F7ED53220B54D600D63515 /* AppDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2F7ED52220B54D600D63515 /* AppDelegate.swift */; }; @@ -103,6 +105,8 @@ E2A17B1200000000000000B2 /* test.ods */ = {isa = PBXFileReference; lastKnownFileType = file; path = test.ods; sourceTree = ""; }; E2A17B1300000000000000B3 /* test.odp */ = {isa = PBXFileReference; lastKnownFileType = file; path = test.odp; sourceTree = ""; }; E26C39382250DC6E009C484A /* WebKit.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = WebKit.framework; path = System/Library/Frameworks/WebKit.framework; sourceTree = SDKROOT; }; + E2A17B2200000000000000C2 /* CoreFoundation.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = CoreFoundation.framework; path = System/Library/Frameworks/CoreFoundation.framework; sourceTree = SDKROOT; }; + E2A17B2300000000000000C3 /* CFNetwork.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = CFNetwork.framework; path = System/Library/Frameworks/CFNetwork.framework; sourceTree = SDKROOT; }; E2BB4B60220EF3A10056176B /* BridgingHeader.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = BridgingHeader.h; sourceTree = ""; }; E2C008F9220F1CF80097C594 /* CoreWrapper.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = CoreWrapper.mm; sourceTree = ""; }; E2C008FC220F1D570097C594 /* CoreWrapper.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CoreWrapper.h; sourceTree = ""; }; @@ -131,6 +135,8 @@ 523A371528CCF28100876C77 /* StoreKit.framework in Frameworks */, E23795302274844400BA7238 /* AdSupport.framework in Frameworks */, E26C39392250DC6E009C484A /* WebKit.framework in Frameworks */, + E2A17B2000000000000000C0 /* CoreFoundation.framework in Frameworks */, + E2A17B2100000000000000C1 /* CFNetwork.framework in Frameworks */, 523A371328CCF27400876C77 /* AdServices.framework in Frameworks */, E1A78CF72C1A53DB00CD43E4 /* Foundation.framework in Frameworks */, D2DCD7104EBCC0F1A784E116 /* GoogleMobileAds in Frameworks */, @@ -209,6 +215,8 @@ E2064DDF22CFA1BA006441F8 /* iAd.framework */, E237952F2274844400BA7238 /* AdSupport.framework */, E26C39382250DC6E009C484A /* WebKit.framework */, + E2A17B2200000000000000C2 /* CoreFoundation.framework */, + E2A17B2300000000000000C3 /* CFNetwork.framework */, ); name = Frameworks; sourceTree = ""; diff --git a/OpenDocumentReader/CoreWrapper.h b/OpenDocumentReader/CoreWrapper.h index 30f171f..eab4ee5 100644 --- a/OpenDocumentReader/CoreWrapper.h +++ b/OpenDocumentReader/CoreWrapper.h @@ -27,7 +27,11 @@ typedef NS_ERROR_ENUM(CoreWrapperErrorDomain, CoreWrapperError) { @interface CoreWrapper : NSObject @property (nonatomic, copy, readonly) NSArray *pageNames; -@property (nonatomic, copy, readonly) NSArray *pagePaths; + +/// One URL per page, in the order they should be shown. Loopback HTTP URLs +/// served by odrcore, or `file:` URLs below the output path when the app is +/// built to render offline - see `kCoreWrapperServesOverHttp`. +@property (nonatomic, copy, readonly) NSArray *pageURLs; - (BOOL)translate:(NSString *)inputPath cache:(NSString *)cachePath diff --git a/OpenDocumentReader/CoreWrapper.mm b/OpenDocumentReader/CoreWrapper.mm index 12aa81a..c5b14bc 100644 --- a/OpenDocumentReader/CoreWrapper.mm +++ b/OpenDocumentReader/CoreWrapper.mm @@ -14,13 +14,35 @@ #include #include #include +#include #include #include #include +#include +#include +#include + #include +#include +#include #include #include +#include + +/// Whether the translated HTML reaches the web view over loopback HTTP instead +/// of being written to the output directory as files. +/// +/// HTTP is what OpenDocument.droid does and where this is headed. odrcore then +/// renders a page when the web view asks for it, on one of the server's +/// threads, rather than rendering every page up front on whichever thread +/// called `translate`. Set this to `NO` to go back to files - the offline path +/// is kept working until the migration is finished, not as a runtime option. +static const BOOL kCoreWrapperServesOverHttp = YES; + +/// Loopback port for that server, the same one OpenDocument.droid uses. +/// Nothing off the device can reach it: the socket is bound to 127.0.0.1. +static const std::uint32_t kCoreWrapperHttpPort = 29665; NSErrorDomain const CoreWrapperErrorDomain = @"app.opendocument.CoreWrapperErrorDomain"; @@ -65,6 +87,92 @@ static bool CoreWrapperIsCombinedView(const odr::HtmlView &view) { return selected; } +/// `listen` binds the socket on the server's own thread, so without this the +/// first request can lose the race and get "connection refused" instead of a +/// document. Returns whether the port answers within `timeout`. +static BOOL CoreWrapperWaitForPort(std::uint32_t port, NSTimeInterval timeout) { + NSDate *deadline = [NSDate dateWithTimeIntervalSinceNow:timeout]; + + while (true) { + int handle = socket(AF_INET, SOCK_STREAM, 0); + if (handle < 0) { + return NO; + } + + struct sockaddr_in address = {}; + address.sin_len = sizeof(address); + address.sin_family = AF_INET; + address.sin_port = htons(static_cast(port)); + address.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + + int connected = connect(handle, reinterpret_cast(&address), sizeof(address)); + close(handle); + + if (connected == 0) { + return YES; + } + if ([deadline timeIntervalSinceNow] <= 0) { + return NO; + } + + usleep(10 * 1000); + } +} + +static std::optional g_server; + +/// Brings up the one server the app has, on first use, and leaves it running +/// for the rest of the process. Returns NO when the port could not be taken - +/// another app may hold it - in which case translate falls back to files. +/// +/// Only attempted once: a port that is taken now is not going to be free on the +/// next document either, and retrying would fork the state of `g_server`. +static BOOL CoreWrapperStartServer() { + static BOOL running = NO; + static dispatch_once_t onceToken; + + dispatch_once(&onceToken, ^{ + NSString *cachePath = [NSTemporaryDirectory() stringByAppendingPathComponent:@"odrcore-server"]; + + odr::HttpServer::Config config; + config.cache_path = std::string([cachePath UTF8String]); + + try { + std::filesystem::create_directories(config.cache_path); + g_server = odr::HttpServer(config); + } catch (...) { + return; + } + + // listen() only returns once the server is stopped, and the server is + // stopped when the process ends, so this thread is never joined + std::thread([] { + try { + g_server->listen("127.0.0.1", kCoreWrapperHttpPort); + } catch (...) { + } + }).detach(); + + running = CoreWrapperWaitForPort(kCoreWrapperHttpPort, 5); + }); + + return running; +} + +/// Where the server serves a view: `HttpServer` routes `/file//` +/// to the service connected under that prefix, and the pages ask for their +/// images and fonts relative to that, so those are covered by the same route. +static NSURL *CoreWrapperPageURL(const std::string &prefix, const std::string &path) { + NSString *escapedPath = [[NSString stringWithUTF8String:path.c_str()] + stringByAddingPercentEncodingWithAllowedCharacters:[NSCharacterSet URLPathAllowedCharacterSet]]; + + NSString *url = [NSString stringWithFormat:@"http://127.0.0.1:%u/file/%s/%@", + static_cast(kCoreWrapperHttpPort), + prefix.c_str(), escapedPath]; + + return [NSURL URLWithString:url]; +} + /// odrcore's data files ship inside the app bundle. The path never changes at /// runtime, so it is set once instead of on every translate call. static void CoreWrapperEnsureDataPath() { @@ -89,15 +197,20 @@ - (BOOL)translate:(NSString *)inputPath error:(NSError **)error { @synchronized(self) { _pageNames = nil; - _pagePaths = nil; + _pageURLs = nil; try { CoreWrapperEnsureDataPath(); _html.reset(); + BOOL overHttp = kCoreWrapperServesOverHttp && CoreWrapperStartServer(); + odr::HtmlConfig config; config.editable = editable; + // resource paths are resolved relative to an output directory, and + // in server mode there is none - odrcore rejects the combination + config.relative_resource_paths = !overHttp; std::string inputPathCpp = std::string([inputPath UTF8String]); @@ -168,17 +281,39 @@ - (BOOL)translate:(NSString *)inputPath return NO; } - _html = service.bring_offline(outputPathCpp, views); - NSMutableArray *pageNames = [[NSMutableArray alloc] init]; - NSMutableArray *pagePaths = [[NSMutableArray alloc] init]; - for (const auto &page : _html->pages()) { - [pageNames addObject:[NSString stringWithUTF8String:page.name.c_str()]]; - [pagePaths addObject:[NSString stringWithUTF8String:page.path.c_str()]]; + NSMutableArray *pageURLs = [[NSMutableArray alloc] init]; + + if (overHttp) { + // a fresh prefix for every translation, because the web view + // caches by URL: re-translating after a password or an edit has + // to end up at an address it has not seen before + static std::atomic translation{0}; + std::string prefix = "odr" + std::to_string(++translation); + + // drops the service of the document shown before this one, whose + // pages nobody is going to ask for again, along with its cache. + // the directory is recreated first because clear() walks it and + // the system is free to empty the temporary directory for us + std::filesystem::create_directories(g_server->config().cache_path); + g_server->clear(); + g_server->connect_service(service, prefix); + + for (const odr::HtmlView &view : views) { + [pageNames addObject:[NSString stringWithUTF8String:view.name().c_str()]]; + [pageURLs addObject:CoreWrapperPageURL(prefix, view.path())]; + } + } else { + _html = service.bring_offline(outputPathCpp, views); + + for (const auto &page : _html->pages()) { + [pageNames addObject:[NSString stringWithUTF8String:page.name.c_str()]]; + [pageURLs addObject:[NSURL fileURLWithPath:[NSString stringWithUTF8String:page.path.c_str()]]]; + } } _pageNames = pageNames; - _pagePaths = pagePaths; + _pageURLs = pageURLs; return YES; } catch (odr::UnknownFileType &) { diff --git a/OpenDocumentReader/Document.swift b/OpenDocumentReader/Document.swift index cbd475e..a32ff73 100644 --- a/OpenDocumentReader/Document.swift +++ b/OpenDocumentReader/Document.swift @@ -13,13 +13,15 @@ protocol DocumentDelegate: AnyObject { enum DocumentError: Error { case getHtml case backTranslate + /// odrcore accepted the document but could not serve one of its pages. + case pageNotServed } class Document: UIDocument { public var result: URL? public var pageNames: [String]? - public var pagePaths: [String]? + public var pageURLs: [URL]? public weak var delegate: DocumentDelegate? public var loadProgress = Progress(totalUnitCount: 5) @@ -28,7 +30,9 @@ class Document: UIDocument { public var page: Int = 0 { didSet { - parse() + // every page of the document already has an address, so turning to + // one is picking a URL rather than translating the file again + showPage() } } public var password: String? { @@ -61,6 +65,7 @@ class Document: UIDocument { loadProgress.completedUnitCount = 2 result = nil + pageURLs = nil delegate?.documentUpdateContent(self) let cachePath = URL(fileURLWithPath: NSTemporaryDirectory()) @@ -92,15 +97,10 @@ class Document: UIDocument { loadProgress.completedUnitCount = loadProgress.totalUnitCount // translate only reports success once it has at least one page - let pagePaths = coreWrapper.pagePaths - let pageNames = coreWrapper.pageNames - self.pagePaths = pagePaths - self.pageNames = pageNames - - // TODO: use all pages instead of just one! - result = URL(fileURLWithPath: pagePaths[min(page, pagePaths.count - 1)]) + pageURLs = coreWrapper.pageURLs + pageNames = coreWrapper.pageNames - delegate?.documentUpdateContent(self) + showPage() if !wasPageCountAnnounced { delegate?.documentPagesChanged(self) @@ -111,6 +111,17 @@ class Document: UIDocument { delegate?.documentLoadingCompleted(self) } + /// Shows the currently selected page, clamped to what the document has: + /// switching to page five of a spreadsheet and then editing it into four + /// sheets should not walk off the end. + private func showPage() { + guard let pageURLs, !pageURLs.isEmpty else { return } + + result = pageURLs[min(max(page, 0), pageURLs.count - 1)] + + delegate?.documentUpdateContent(self) + } + override func handleError(_ error: Error, userInteractionPermitted: Bool) { CrashManager.shared.log(error) } diff --git a/OpenDocumentReader/DocumentViewController.swift b/OpenDocumentReader/DocumentViewController.swift index eff60e8..feb5f64 100644 --- a/OpenDocumentReader/DocumentViewController.swift +++ b/OpenDocumentReader/DocumentViewController.swift @@ -13,7 +13,9 @@ import UIKit.UIPrinter import WebKit // taken from: https://developer.apple.com/documentation/uikit/view_controllers/building_a_document_browser-based_app -class DocumentViewController: UIViewController, DocumentDelegate, BannerViewDelegate, UISearchBarDelegate { +class DocumentViewController: UIViewController, DocumentDelegate, BannerViewDelegate, UISearchBarDelegate, + WKNavigationDelegate +{ private var browserTransition: DocumentBrowserTransitioningDelegate? public var transitionController: UIDocumentBrowserTransitionController? { @@ -70,6 +72,33 @@ class DocumentViewController: UIViewController, DocumentDelegate, BannerViewDele // once, not on every appearance: a second target would parse the // document twice for a single tap pageTabBar.addTarget(self, action: #selector(pageSelected(sender:)), for: .valueChanged) + + webview.navigationDelegate = self + } + + /// odrcore renders a page when this web view asks for it, so a document + /// that only falls over halfway through translating falls over here rather + /// than in `translate`. Route that into the same handling, which shows the + /// error page or the raw file, instead of letting the server's plain text + /// "Internal Server Error" through. + func webView( + _ webView: WKWebView, decidePolicyFor navigationResponse: WKNavigationResponse, + decisionHandler: @escaping (WKNavigationResponsePolicy) -> Void + ) { + guard let response = navigationResponse.response as? HTTPURLResponse, + response.statusCode >= 400, + let doc = document + else { + decisionHandler(.allow) + + return + } + + decisionHandler(.cancel) + + // the fallback loads a file or an HTML string, neither of which comes + // back as an HTTP response, so this cannot recurse + documentLoadingError(doc, error: DocumentError.pageNotServed) } override func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) { @@ -429,14 +458,20 @@ class DocumentViewController: UIViewController, DocumentDelegate, BannerViewDele } func documentUpdateContent(_ doc: Document) { - guard let path = document?.result else { + guard let url = document?.result else { self.webview.loadHTMLString( "

\(NSLocalizedString("loading", comment: ""))

", baseURL: nil) return } - self.webview.loadFileURL(path, allowingReadAccessTo: path) + // odrcore serves the pages over loopback; the file variant is what the + // app falls back to when that server could not be brought up + if url.isFileURL { + self.webview.loadFileURL(url, allowingReadAccessTo: url) + } else { + self.webview.load(URLRequest(url: url)) + } } func documentEncrypted(_ doc: Document) { diff --git a/OpenDocumentReader/Info-Lite.plist b/OpenDocumentReader/Info-Lite.plist index 1692a29..0fa9cec 100644 --- a/OpenDocumentReader/Info-Lite.plist +++ b/OpenDocumentReader/Info-Lite.plist @@ -56,6 +56,13 @@ ca-app-pub-8161473686436957~5688952247 LSRequiresIPhoneOS + NSAppTransportSecurity + + + NSAllowsLocalNetworking + + NSPhotoLibraryAddUsageDescription Copy image from document to your Photos NSUserTrackingUsageDescription diff --git a/OpenDocumentReader/Info.plist b/OpenDocumentReader/Info.plist index d32046b..9bedcdf 100644 --- a/OpenDocumentReader/Info.plist +++ b/OpenDocumentReader/Info.plist @@ -56,6 +56,13 @@ ca-app-pub-3940256099942544~1458002511 LSRequiresIPhoneOS + NSAppTransportSecurity + + + NSAllowsLocalNetworking + + NSPhotoLibraryAddUsageDescription Copy image from document to your Photos UIApplicationSceneManifest diff --git a/OpenDocumentReaderTests/OpenDocumentReaderTests.swift b/OpenDocumentReaderTests/OpenDocumentReaderTests.swift index 772ffeb..51b9321 100644 --- a/OpenDocumentReaderTests/OpenDocumentReaderTests.swift +++ b/OpenDocumentReaderTests/OpenDocumentReaderTests.swift @@ -6,6 +6,7 @@ // Copyright © 2020 Thomas Taschauer. All rights reserved. // +import WebKit import XCTest @testable import OpenDocumentReader @@ -50,11 +51,8 @@ class OpenDocumentReaderTests: XCTestCase { try wrapper.translate(saveURL.path, cache: cache, into: output, with: nil, editable: true) - XCTAssertFalse(wrapper.pagePaths.isEmpty) - XCTAssertEqual(wrapper.pagePaths.count, wrapper.pageNames.count) - for path in wrapper.pagePaths { - XCTAssertTrue(FileManager.default.fileExists(atPath: path), "missing page at \(path)") - } + XCTAssertFalse(wrapper.pageURLs.isEmpty) + XCTAssertEqual(wrapper.pageURLs.count, wrapper.pageNames.count) } /// A text document has nothing but its combined view. @@ -88,18 +86,74 @@ class OpenDocumentReaderTests: XCTestCase { XCTAssertEqual(wrapper.pageNames, ["document"]) } - /// Views that are not shown must not be rendered either. - func testDiscardedPagesAreNotWrittenOut() throws { - let (wrapper, cache, _) = makeWrapper() - let output = NSTemporaryDirectory() + "presentation-output" - try? FileManager.default.removeItem(atPath: output) - let url = try copyFixture(ofType: "odp") + /// Nothing is rendered up front any more, so the pages have to come back + /// from the loopback server odrcore is serving them on. + func testPagesAreServedOverLoopback() throws { + let (wrapper, cache, output) = makeWrapper() + let url = try copyFixture(ofType: "ods") try wrapper.translate(url.path, cache: cache, into: output, with: nil, editable: false) - let written = try FileManager.default.contentsOfDirectory(atPath: output) - .filter { $0.hasSuffix(".html") } - XCTAssertEqual(written, ["document.html"]) + for pageURL in wrapper.pageURLs { + XCTAssertEqual(pageURL.scheme, "http") + + let (data, response) = try fetch(pageURL) + XCTAssertEqual((response as? HTTPURLResponse)?.statusCode, 200, "\(pageURL)") + XCTAssertTrue( + String(decoding: data, as: UTF8.self).contains(" (Data, URLResponse) { + var result: Result<(Data, URLResponse), Error> = .failure(URLError(.timedOut)) + let done = expectation(description: "GET \(url)") + + URLSession.shared.dataTask(with: url) { data, response, error in + if let data, let response { + result = .success((data, response)) + } else if let error { + result = .failure(error) + } + + done.fulfill() + }.resume() + + wait(for: [done], timeout: 30) + + return try result.get() } func testBackTranslateWritesEditedDocument() throws { @@ -157,3 +211,36 @@ class OpenDocumentReaderTests: XCTestCase { } } } + +/// Fulfills its expectation once, whichever way the navigation ends. +private class NavigationRecorder: NSObject, WKNavigationDelegate { + let finished: XCTestExpectation + private(set) var error: Error? + private var isDone = false + + init(finished: XCTestExpectation) { + self.finished = finished + } + + private func complete(_ error: Error?) { + guard !isDone else { return } + + isDone = true + self.error = error + finished.fulfill() + } + + func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) { + complete(nil) + } + + func webView(_ webView: WKWebView, didFail navigation: WKNavigation!, withError error: Error) { + complete(error) + } + + func webView( + _ webView: WKWebView, didFailProvisionalNavigation navigation: WKNavigation!, withError error: Error + ) { + complete(error) + } +} diff --git a/README.md b/README.md index 38e894b..4ce65d3 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,35 @@ Everything else comes from Swift Package Manager and is resolved by Xcode. `conan/setup-all.sh` has to be re-run whenever the odrcore version in `conan/conanfile.py` or the `conan-odr-index` submodule changes. +## How a document reaches the screen + +`CoreWrapper` hands the file to odrcore, which returns an `HtmlService`: a +handle that knows which views the document has but has not rendered any of +them. That service is connected to odrcore's HTTP server, bound to +`127.0.0.1:29665`, and the web view is pointed at +`http://127.0.0.1:29665/file//.html`. odrcore renders a page when +the web view asks for it, on one of the server's threads. + +The same thing OpenDocument.droid does, and for the same reasons: rendering +happens off the thread that opened the document, only the pages that are looked +at are rendered at all, and turning a page is a navigation rather than another +translation. The `` changes on every translation, because the web view +caches by URL and a document re-translated after a password or an edit has to +land on an address it has not seen. + +`kCoreWrapperServesOverHttp` in `CoreWrapper.mm` switches back to writing the +pages out as files, which is also what happens automatically if the port cannot +be bound. It is there to keep that path working while the migration finishes, +not as a runtime option. + +Nothing about this needs a capability or prompts the user. A listening socket on +loopback takes no entitlement, and the local network permission introduced in +iOS 14 covers the local subnet and multicast, not `127.0.0.1`. The one thing it +does need is an App Transport Security exception, since ATS blocks plain HTTP: +`NSAllowsLocalNetworking` in both `Info.plist`s, which is the narrow one for +local addresses and — unlike `NSAllowsArbitraryLoads` — needs no justification +in App Store review. + ## Formatting Swift sources are formatted with `swift-format` from the active Xcode diff --git a/conan/conanfile.py b/conan/conanfile.py index 2c94c7e..c0e2208 100644 --- a/conan/conanfile.py +++ b/conan/conanfile.py @@ -10,7 +10,7 @@ class Pkg(ConanFile): "odrcore/*:with_pdf2htmlEX": False, "odrcore/*:with_wvWare": False, "odrcore/*:with_libmagic": False, - "odrcore/*:with_http_server": False, + "odrcore/*:with_http_server": True, } requires = "odrcore/5.5.0" diff --git a/conan/profiles/ios-base b/conan/profiles/ios-base index 0f623a9..e8ab16c 100644 --- a/conan/profiles/ios-base +++ b/conan/profiles/ios-base @@ -10,3 +10,12 @@ compiler.cppstd=20 # reports from the field stay symbolicatable build_type=Release odrcore/*:build_type=RelWithDebInfo + +[conf] +# with the http server enabled, cpp-httplib resolves host names through CFHost +# on Apple platforms. odrcore's CMake links neither CoreFoundation nor CFNetwork +# (where CFHost lives), so its `cli/server` tool fails to link - the library +# itself builds fine, and the app links both frameworks itself. Only executables +# need this, hence exelinkflags. (`ODR_CLI=OFF` would be the tidier fix but +# odrcore 5.5.0 installs the cli targets whether or not it built them.) +odrcore/*:tools.build:exelinkflags=["-framework", "CoreFoundation", "-framework", "CFNetwork"] From 6d2a3ccc789ad55b61edb5dbe794b69c77d8ff57 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 26 Jul 2026 12:23:31 +0200 Subject: [PATCH 2/2] Confirm the loopback socket is ours, and only ours can fail a page Two things the review caught. The readiness probe connected to the port and took an answer as proof that the server was up. If another app already held 29665, our listen() failed, its exception was swallowed, and the probe happily shook hands with the stranger - after which every document got URLs pointing at a server that had never heard of it, for the rest of the process. The port is now bind-tested before the server is started, which is what actually answers "is anyone else here", and listen() coming back sets a flag the wait loop checks, covering the case where somebody takes the port in between. The 4xx/5xx handling applied to every response the web view saw, so a link in the document leading to a 404, or a frame inside it failing, replaced the document with the error page. It now only applies to the main frame and only to URLs odrcore is serving, which `CoreWrapper` answers for since it owns the host and port. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011MhKU2kWm1cPW4GBq9gon5 --- OpenDocumentReader/CoreWrapper.h | 4 + OpenDocumentReader/CoreWrapper.mm | 98 +++++++++++++++---- .../DocumentViewController.swift | 8 +- .../OpenDocumentReaderTests.swift | 21 ++++ 4 files changed, 110 insertions(+), 21 deletions(-) diff --git a/OpenDocumentReader/CoreWrapper.h b/OpenDocumentReader/CoreWrapper.h index eab4ee5..3bcf757 100644 --- a/OpenDocumentReader/CoreWrapper.h +++ b/OpenDocumentReader/CoreWrapper.h @@ -44,6 +44,10 @@ typedef NS_ERROR_ENUM(CoreWrapperErrorDomain, CoreWrapperError) { into:(NSString *)outputPath error:(NSError **)error; +/// Whether this URL is one odrcore is serving, rather than somewhere a link in +/// the document leads. ++ (BOOL)isServedURL:(NSURL *)url; + @end NS_ASSUME_NONNULL_END diff --git a/OpenDocumentReader/CoreWrapper.mm b/OpenDocumentReader/CoreWrapper.mm index c5b14bc..f207504 100644 --- a/OpenDocumentReader/CoreWrapper.mm +++ b/OpenDocumentReader/CoreWrapper.mm @@ -87,28 +87,75 @@ static bool CoreWrapperIsCombinedView(const odr::HtmlView &view) { return selected; } -/// `listen` binds the socket on the server's own thread, so without this the +static struct sockaddr_in CoreWrapperLoopbackAddress(std::uint32_t port) { + struct sockaddr_in address = {}; + address.sin_len = sizeof(address); + address.sin_family = AF_INET; + address.sin_port = htons(static_cast(port)); + address.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + + return address; +} + +/// Whether the port is free, asked by binding it and letting go again. +/// +/// Without this, a port some other app is already listening on would answer the +/// readiness probe below and the app would spend the rest of its life handing +/// the web view addresses on a server that knows nothing about our documents. +/// +/// `SO_REUSEADDR` matches what cpp-httplib sets, so a connection of ours still +/// in TIME_WAIT does not read as somebody else's port. `SO_REUSEPORT`, which +/// httplib also sets, is deliberately not: it would let this bind succeed +/// alongside a foreign listener, which is the case being ruled out. +static BOOL CoreWrapperPortIsFree(std::uint32_t port) { + int handle = socket(AF_INET, SOCK_STREAM, 0); + if (handle < 0) { + return NO; + } + + int reuse = 1; + setsockopt(handle, SOL_SOCKET, SO_REUSEADDR, &reuse, sizeof(reuse)); + + struct sockaddr_in address = CoreWrapperLoopbackAddress(port); + int bound = bind(handle, reinterpret_cast(&address), sizeof(address)); + close(handle); + + return bound == 0; +} + +static BOOL CoreWrapperPortAnswers(std::uint32_t port) { + int handle = socket(AF_INET, SOCK_STREAM, 0); + if (handle < 0) { + return NO; + } + + struct sockaddr_in address = CoreWrapperLoopbackAddress(port); + int connected = connect(handle, reinterpret_cast(&address), sizeof(address)); + close(handle); + + return connected == 0; +} + +static std::optional g_server; + +/// Set when `listen` comes back, which it only does when the bind failed or the +/// server was stopped. While it is clear, the socket on the port is ours. +static std::atomic g_serverStopped{false}; + +/// `listen` binds the socket on the server's own thread, so without waiting the /// first request can lose the race and get "connection refused" instead of a -/// document. Returns whether the port answers within `timeout`. -static BOOL CoreWrapperWaitForPort(std::uint32_t port, NSTimeInterval timeout) { +/// document. +static BOOL CoreWrapperWaitForServer(std::uint32_t port, NSTimeInterval timeout) { NSDate *deadline = [NSDate dateWithTimeIntervalSinceNow:timeout]; while (true) { - int handle = socket(AF_INET, SOCK_STREAM, 0); - if (handle < 0) { + if (g_serverStopped.load()) { return NO; } - - struct sockaddr_in address = {}; - address.sin_len = sizeof(address); - address.sin_family = AF_INET; - address.sin_port = htons(static_cast(port)); - address.sin_addr.s_addr = htonl(INADDR_LOOPBACK); - - int connected = connect(handle, reinterpret_cast(&address), sizeof(address)); - close(handle); - - if (connected == 0) { + // re-checked afterwards: a port that answers between our bind test and + // this point belongs to whoever won the race, and if that is not us + // then listen() has come back by now + if (CoreWrapperPortAnswers(port) && !g_serverStopped.load()) { return YES; } if ([deadline timeIntervalSinceNow] <= 0) { @@ -119,8 +166,6 @@ static BOOL CoreWrapperWaitForPort(std::uint32_t port, NSTimeInterval timeout) { } } -static std::optional g_server; - /// Brings up the one server the app has, on first use, and leaves it running /// for the rest of the process. Returns NO when the port could not be taken - /// another app may hold it - in which case translate falls back to files. @@ -132,6 +177,10 @@ static BOOL CoreWrapperStartServer() { static dispatch_once_t onceToken; dispatch_once(&onceToken, ^{ + if (!CoreWrapperPortIsFree(kCoreWrapperHttpPort)) { + return; + } + NSString *cachePath = [NSTemporaryDirectory() stringByAppendingPathComponent:@"odrcore-server"]; odr::HttpServer::Config config; @@ -151,14 +200,18 @@ static BOOL CoreWrapperStartServer() { g_server->listen("127.0.0.1", kCoreWrapperHttpPort); } catch (...) { } + + g_serverStopped.store(true); }).detach(); - running = CoreWrapperWaitForPort(kCoreWrapperHttpPort, 5); + running = CoreWrapperWaitForServer(kCoreWrapperHttpPort, 5); }); return running; } +static NSString *const kCoreWrapperHttpHost = @"127.0.0.1"; + /// Where the server serves a view: `HttpServer` routes `/file//` /// to the service connected under that prefix, and the pages ask for their /// images and fonts relative to that, so those are covered by the same route. @@ -166,7 +219,7 @@ static BOOL CoreWrapperStartServer() { NSString *escapedPath = [[NSString stringWithUTF8String:path.c_str()] stringByAddingPercentEncodingWithAllowedCharacters:[NSCharacterSet URLPathAllowedCharacterSet]]; - NSString *url = [NSString stringWithFormat:@"http://127.0.0.1:%u/file/%s/%@", + NSString *url = [NSString stringWithFormat:@"http://%@:%u/file/%s/%@", kCoreWrapperHttpHost, static_cast(kCoreWrapperHttpPort), prefix.c_str(), escapedPath]; @@ -337,6 +390,11 @@ - (BOOL)translate:(NSString *)inputPath } } ++ (BOOL)isServedURL:(NSURL *)url { + return [url.scheme isEqualToString:@"http"] && [url.host isEqualToString:kCoreWrapperHttpHost] + && url.port.unsignedIntValue == kCoreWrapperHttpPort; +} + - (BOOL)backTranslate:(NSString *)diff into:(NSString *)outputPath error:(NSError **)error { @synchronized(self) { if (!_document.has_value()) { diff --git a/OpenDocumentReader/DocumentViewController.swift b/OpenDocumentReader/DocumentViewController.swift index feb5f64..04ddc34 100644 --- a/OpenDocumentReader/DocumentViewController.swift +++ b/OpenDocumentReader/DocumentViewController.swift @@ -81,12 +81,18 @@ class DocumentViewController: UIViewController, DocumentDelegate, BannerViewDele /// than in `translate`. Route that into the same handling, which shows the /// error page or the raw file, instead of letting the server's plain text /// "Internal Server Error" through. + /// + /// Only for the page itself: a link in the document leading somewhere that + /// answers 404, or a frame inside it doing so, is not this document failing + /// to render and must not replace it. func webView( _ webView: WKWebView, decidePolicyFor navigationResponse: WKNavigationResponse, decisionHandler: @escaping (WKNavigationResponsePolicy) -> Void ) { - guard let response = navigationResponse.response as? HTTPURLResponse, + guard navigationResponse.isForMainFrame, + let response = navigationResponse.response as? HTTPURLResponse, response.statusCode >= 400, + let url = response.url, CoreWrapper.isServedURL(url), let doc = document else { decisionHandler(.allow) diff --git a/OpenDocumentReaderTests/OpenDocumentReaderTests.swift b/OpenDocumentReaderTests/OpenDocumentReaderTests.swift index 51b9321..ea69660 100644 --- a/OpenDocumentReaderTests/OpenDocumentReaderTests.swift +++ b/OpenDocumentReaderTests/OpenDocumentReaderTests.swift @@ -137,6 +137,27 @@ class OpenDocumentReaderTests: XCTestCase { XCTAssertNil(recorder.error) } + /// A failing page is treated as the document failing to render, so what + /// counts as one of our pages has to be narrow: a link in the document that + /// answers 404 must not take the document down with it. + func testOnlyTheServersOwnURLsCountAsPages() throws { + let (wrapper, cache, output) = makeWrapper() + + try wrapper.translate(saveURL.path, cache: cache, into: output, with: nil, editable: false) + + let page = try XCTUnwrap(wrapper.pageURLs.first) + XCTAssertTrue(CoreWrapper.isServedURL(page)) + + for other in [ + "https://opendocument.app/missing.html", + "http://opendocument.app/missing.html", + "http://127.0.0.1:8080/file/odr1/document.html", + "http://localhost:29665/file/odr1/document.html", + ] { + XCTAssertFalse(CoreWrapper.isServedURL(try XCTUnwrap(URL(string: other))), other) + } + } + private func fetch(_ url: URL) throws -> (Data, URLResponse) { var result: Result<(Data, URLResponse), Error> = .failure(URLError(.timedOut)) let done = expectation(description: "GET \(url)")