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)