From 20f745c49a7ff9bb0f15bc92939b660a9010af71 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 28 Jul 2026 00:35:22 +0200 Subject: [PATCH] Update to odrcore 6.0.0 conan-odr-index moves to 36caf08, which adds the odrcore/6 recipe. 6.0.0 drops the pdf2htmlEX and wvWare backends and reworks the HTTP server API, so the consumer side has to move with it: - `with_pdf2htmlEX` and `with_wvWare` are gone from the recipe, and both were already off here. pdf2htmlex, poppler-data and fontconfig leave the dependency graph with them, so the deployer branches copying their data into the bundle are dead and go too - odrcore's own data is all that is left. - `HttpServer::listen(host, port)` is split into `bind()` + `listen()`. bind() runs on the calling thread, reports a failed bind as ServerBindFailed instead of swallowing it, and returns the port it got. That removes the reason for every socket call in CoreWrapper: the bind test that checked the port was not somebody else's, and the readiness probe that waited for listen() to bind on the server's own thread. Connections are accepted from bind() onwards. It also removes the reason for a fixed port. Binding port 0 asks for whichever one is free and bind() says which that was, so the two flavors of the app can no longer collide on 29665 - the failure this API change was made for. The port is kept in a global, since `isServedURL:` and the page URLs both need it. - `HttpServer::Config::cache_path` and `config()` are gone, and clear() no longer deletes files. Nothing is lost: the pages were translated into the cache path passed to `html::translate`, which is the system's temporary directory, and the server's own cache directory was only ever used by the `serve_file` that 6.0.0 removed. The port in the tests is now derived from the served URL rather than hardcoded, with a bare-host case added since a URL without a port no longer reads as "not ours" for free. Verified with `conan/setup-all.sh` across all 13 configurations, `fastlane tests` (27 passing, including the loopback and web view ones), and device Release builds of both flavors. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018nJRmG9vePpssqA3jnfNv9 --- OpenDocumentReader/CoreWrapper.mm | 141 ++++-------------- .../OpenDocumentReaderTests.swift | 9 +- README.md | 12 +- conan-odr-index | 2 +- conan/conandeployer.py | 29 +--- conan/conanfile.py | 4 +- 6 files changed, 47 insertions(+), 150 deletions(-) diff --git a/OpenDocumentReader/CoreWrapper.mm b/OpenDocumentReader/CoreWrapper.mm index f207504..c989c23 100644 --- a/OpenDocumentReader/CoreWrapper.mm +++ b/OpenDocumentReader/CoreWrapper.mm @@ -19,13 +19,8 @@ #include #include -#include -#include -#include - #include #include -#include #include #include #include @@ -40,9 +35,8 @@ /// 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; +/// Loopback address for that server. Nothing off the device can reach it. +static NSString *const kCoreWrapperHttpHost = @"127.0.0.1"; NSErrorDomain const CoreWrapperErrorDomain = @"app.opendocument.CoreWrapperErrorDomain"; @@ -87,131 +81,52 @@ static bool CoreWrapperIsCombinedView(const odr::HtmlView &view) { return selected; } -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. -static BOOL CoreWrapperWaitForServer(std::uint32_t port, NSTimeInterval timeout) { - NSDate *deadline = [NSDate dateWithTimeIntervalSinceNow:timeout]; - - while (true) { - if (g_serverStopped.load()) { - return NO; - } - // 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) { - return NO; - } - - usleep(10 * 1000); - } -} +/// The port `bind` handed us, and with it whether there is a server at all. +/// Zero until one is running. +static std::atomic g_serverPort{0}; /// 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. +/// for the rest of the process. Returns NO when the socket could not be opened, +/// 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`. +/// Only attempted once: retrying would fork the state of `g_server`. static BOOL CoreWrapperStartServer() { static BOOL running = NO; static dispatch_once_t onceToken; dispatch_once(&onceToken, ^{ - if (!CoreWrapperPortIsFree(kCoreWrapperHttpPort)) { - return; - } - - 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); + g_server.emplace(); + // port 0 asks for whichever port is free and bind() reports back + // which one that was. Nothing outside this process needs to know it + // in advance, and a fixed port would be one the other flavor of the + // app, or anything else on the device, could already be holding + g_serverPort.store(g_server->bind([kCoreWrapperHttpHost UTF8String], 0)); } catch (...) { + g_server.reset(); + return; } + // the socket is bound and taking connections into its backlog from here + // on, so there is nothing to wait for before handing out URLs. // 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); + g_server->listen(); } catch (...) { } - - g_serverStopped.store(true); }).detach(); - running = CoreWrapperWaitForServer(kCoreWrapperHttpPort, 5); + running = YES; }); 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. @@ -220,7 +135,7 @@ static BOOL CoreWrapperStartServer() { stringByAddingPercentEncodingWithAllowedCharacters:[NSCharacterSet URLPathAllowedCharacterSet]]; NSString *url = [NSString stringWithFormat:@"http://%@:%u/file/%s/%@", kCoreWrapperHttpHost, - static_cast(kCoreWrapperHttpPort), + static_cast(g_serverPort.load()), prefix.c_str(), escapedPath]; return [NSURL URLWithString:url]; @@ -345,10 +260,9 @@ - (BOOL)translate:(NSString *)inputPath 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); + // pages nobody is going to ask for again. what it was translated + // into stays in the cache directory, which is the system's to + // empty rather than the server's g_server->clear(); g_server->connect_service(service, prefix); @@ -391,8 +305,11 @@ - (BOOL)translate:(NSString *)inputPath } + (BOOL)isServedURL:(NSURL *)url { - return [url.scheme isEqualToString:@"http"] && [url.host isEqualToString:kCoreWrapperHttpHost] - && url.port.unsignedIntValue == kCoreWrapperHttpPort; + std::uint32_t port = g_serverPort.load(); + + return port != 0 && [url.scheme isEqualToString:@"http"] + && [url.host isEqualToString:kCoreWrapperHttpHost] + && url.port.unsignedIntValue == port; } - (BOOL)backTranslate:(NSString *)diff into:(NSString *)outputPath error:(NSError **)error { diff --git a/OpenDocumentReaderTests/OpenDocumentReaderTests.swift b/OpenDocumentReaderTests/OpenDocumentReaderTests.swift index ea69660..2a7f9cc 100644 --- a/OpenDocumentReaderTests/OpenDocumentReaderTests.swift +++ b/OpenDocumentReaderTests/OpenDocumentReaderTests.swift @@ -148,11 +148,16 @@ class OpenDocumentReaderTests: XCTestCase { let page = try XCTUnwrap(wrapper.pageURLs.first) XCTAssertTrue(CoreWrapper.isServedURL(page)) + // the port is whichever one was free, so the near misses are spelled + // relative to the one we actually got rather than a hardcoded number + let port = try XCTUnwrap(page.port) + 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", + "http://127.0.0.1:\(port + 1)/file/odr1/document.html", + "http://127.0.0.1/file/odr1/document.html", + "http://localhost:\(port)/file/odr1/document.html", ] { XCTAssertFalse(CoreWrapper.isServedURL(try XCTUnwrap(URL(string: other))), other) } diff --git a/README.md b/README.md index 4ce65d3..9bb8984 100644 --- a/README.md +++ b/README.md @@ -30,9 +30,9 @@ Everything else comes from Swift Package Manager and is resolved by Xcode. `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 +them. That service is connected to odrcore's HTTP server, bound to `127.0.0.1` +on whichever port was free, and the web view is pointed at +`http://127.0.0.1:/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 @@ -43,9 +43,9 @@ 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. +pages out as files, which is also what happens automatically if the socket +cannot be opened at all. 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 diff --git a/conan-odr-index b/conan-odr-index index 010a878..36caf08 160000 --- a/conan-odr-index +++ b/conan-odr-index @@ -1 +1 @@ -Subproject commit 010a87833ee98f67163d07c459a7ed6cf088d940 +Subproject commit 36caf08e2de1327132c7b19b57ba33b3be66264d diff --git a/conan/conandeployer.py b/conan/conandeployer.py index 63eceb9..d000f5a 100644 --- a/conan/conandeployer.py +++ b/conan/conandeployer.py @@ -34,32 +34,9 @@ def deploy(graph, output_folder: str, **kwargs): **copytree_kwargs, ) - if "pdf2htmlex" in deps: - dep = deps["pdf2htmlex"] - conanfile.output.info(f"Deploying pdf2htmlex to {conan_files}") - shutil.copytree( - f"{dep.package_folder}/share/pdf2htmlEX", - f"{conan_files}/pdf2htmlex", - **copytree_kwargs, - ) - - if "poppler-data" in deps: - dep = deps["poppler-data"] - conanfile.output.info(f"Deploying poppler-data to {conan_files}") - shutil.copytree( - f"{dep.package_folder}/share/poppler", - f"{conan_files}/poppler", - **copytree_kwargs, - ) - - if "fontconfig" in deps: - dep = deps["fontconfig"] - conanfile.output.info(f"Deploying fontconfig to {conan_files}") - shutil.copytree( - f"{dep.package_folder}/res/share", - f"{conan_files}/fontconfig", - **copytree_kwargs, - ) + # odrcore 6 dropped the pdf2htmlEX and wvWare backends, so pdf2htmlex, + # poppler-data and fontconfig are no longer in the graph and nothing but + # odrcore's own data is left to deploy with ( open(f"{output_folder}/input-files.xcfilelist", "w") as f_in, diff --git a/conan/conanfile.py b/conan/conanfile.py index 38d0d3e..92563e4 100644 --- a/conan/conanfile.py +++ b/conan/conanfile.py @@ -7,13 +7,11 @@ class Pkg(ConanFile): default_options = { "configuration": "Debug", "odrcore/*:shared": False, - "odrcore/*:with_pdf2htmlEX": False, - "odrcore/*:with_wvWare": False, "odrcore/*:with_libmagic": False, "odrcore/*:with_http_server": True, "odrcore/*:with_cli": False, } - requires = "odrcore/5.7.0" + requires = "odrcore/6.0.0" def generate(self): xcode = XcodeDeps(self)