Serve the translated document over loopback instead of files - #125
Merged
Conversation
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011MhKU2kWm1cPW4GBq9gon5
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e02b7e2c79
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011MhKU2kWm1cPW4GBq9gon5
This was referenced Jul 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Generated with Claude Code
Why
odrcore rendered every page of a document before any of them was shown, on whichever thread called
translate— the main thread whenever a password is entered or editing is toggled.odr::HttpServer, which OpenDocument.droid already uses, turns that around:odr::html::translatereturns anHtmlServicethat has rendered nothing127.0.0.1:29665http://127.0.0.1:29665/file/<prefix>/<page>.htmlSo a page nobody opens is never rendered, the work that does happen is off the calling thread, and turning a page is a navigation rather than another translation —
Document.pageno longer re-parses the file to show a page whose address it already had.The
<prefix>changes on every translation: 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.Permissions — the question that was asked
Nothing here needs a capability or prompts the user:
127.0.0.1Info.plists gainNSAllowsLocalNetworking— the narrow exception for local addresses, which unlikeNSAllowsArbitraryLoadsneeds no justification in App Store reviewtestTheWebViewLoadsAServedPagecovers exactly this: it drives a realWKWebViewat a served page, which is the only place a missing ATS exception would show up (URLSessionis happy either way).Trade-off worth knowing
A document that only falls over halfway through rendering now fails while the page is being served rather than inside
translate, so that failure is no longer caught there.DocumentViewControllertherefore acts as the web view's navigation delegate and routes a 4xx/5xx back intodocumentLoadingError, so the user still gets the error page or the raw file rather than the server's plain text "Internal Server Error".Escape hatch
kCoreWrapperServesOverHttpinCoreWrapper.mmswitches back to writing the pages out as files, which is also what happens by itself when the port cannot be bound. It exists to keep that path working while the migration finishes, not as a runtime option.Build changes
conan/conanfile.py:odrcore/*:with_http_server=Trueconan/profiles/ios-base:-framework CoreFoundation -framework CFNetworkfor odrcore's executables. cpp-httplib resolves host names throughCFHoston Apple platforms and odrcore's CMake links neither framework, so itscli/servertool fails to link. (ODR_CLI=OFFwould be tidier but odrcore 5.5.0 installs the cli targets whether or not it built them — worth fixing upstream.)CoreFoundationandCFNetworkfor the same reasonHtmlConfig.relative_resource_pathsis off in server mode; odrcore rejects the combination, since there is no output directory to be relative toconan/setup-all.shhas to be re-run — odrcore is rebuilt with the http server.Verified
xcodebuild teston the iPhone 17 simulator (26 tests, all pass) and a device build of the Lite/Release slice.