feat(http-server)!: split listen into bind and listen

[andiwand]

🤖 Generated with Claude Code

HttpServer::listen(host, port) takes the address, blocks, and returns void — discarding the bool cpp-httplib hands it:

void listen(const std::string &host, const std::uint32_t port) const {
  ODR_VERBOSE(*m_logger, "Listening on " << host << ":" << port);
  m_server->listen(host, static_cast<int>(port));   // returns bool, dropped
}

So a failed bind is known inside the library and thrown away twice, and the caller is left with a server that never listened and no way to find out.

That is not hypothetical. OpenDocument.droid hit it in opendocument-app/OpenDocument.droid#536: both flavors hardcode port 29665, connectedCheck runs them one after the other on one device, and the second app's bind failed against the first's TIME_WAIT sockets. No server, no complaint, and every document silently became chrome's error page — which read as "edit mode is broken" for three rounds of CI archaeology until a WebViewClient.onReceivedError finally said net::ERR_CONNECTION_REFUSED. The app now probes the port with a throwaway Java socket before starting the server, which is a guess about how the native bind behaves, and racy. This removes the need for it.

What changed

Config is empty, serve_file and config() are gone, and listen(host, port) became bind() + listen().

The split

/// Binds the socket and returns the port it got - pass port 0 for any free
/// one. Connections are accepted into the backlog from here on, before
/// listen() runs. Throws ServerBindFailed / ServerAlreadyBound.
std::uint32_t bind(const std::string &host, std::uint32_t port,
                   const Options &options = {}) const;

/// Serves what bind() opened until stop(). Throws ServerNotBound.
void listen() const;

Config and Options sit at namespace scope, next to HtmlConfig which is spelled that way already, and the class keeps them as aliases (HttpServer::Config still works). That is what lets both be defaulted with = {}: a nested class's member initializers are not parsed until the enclosing class is complete, so a default argument inside the class cannot reach them. Hence one bind() rather than two overloads, and HttpServer server; with no arguments at all.

Three things follow from it:

• failure is reported — ServerBindFailed carries the host and port
• port 0 becomes usable — bind returns what the OS gave, so no caller has to pick a free port and hope it stays free
• no "is it up yet?" window — cpp-httplib calls ::listen(sock, backlog) inside the bind, so connections queue from the moment bind() returns, before the accept loop starts

Two states cpp-httplib handles silently, now errors

• binding twice overwrote svr_sock_ without closing it, leaking the first socket and holding its port until the process ended → ServerAlreadyBound
• listen before bind: listen_internal() sets ret = true, finds svr_sock_ == INVALID_SOCKET so the accept loop never runs, and returns true. A bool return would have reported success for the one case a caller most needs to hear about → ServerNotBound

The second is why listen() throws rather than returning bool.

Config is empty, and the server no longer owns a cache

Config held nothing but cache_path, which existed only for the serve_file marked // TODO remove — and for a clear() that deleted the caller's translated files underneath them. Both are gone. The server hosts what it is given; what a service was translated into belongs to whoever translated it, and clear() now only drops the connected services.

Callers translate for themselves, which is what serve_file did internally anyway:

const HtmlService service = html::translate(file, my_cache_path, html_config, logger);
server.connect_service(service, prefix);

Config stays as an empty struct, so server-wide settings have somewhere to land later.

Socket options belong to the bind

HttpServer::Options is passed to bind(), not held on the server, and it corrects a default:

	cpp-httplib	here
SO_REUSEADDR	only where SO_REUSEPORT is undefined	on
SO_REUSEPORT	on, wherever it exists	off

Only SO_REUSEADDR lets a port still held by TIME_WAIT sockets be bound again — the failure above. SO_REUSEPORT does not help with that at all, and what it does do is let a second live server share the port and take a random share of the connections, which for a document server means requests answered by the wrong instance. It also only shares between sockets of the same uid, so it was never going to help two Android flavors either.

Windows keeps cpp-httplib's defaults (thanks to the review bot for catching this). The two flags mean the opposite there: SO_REUSEADDR lets a second live socket take the endpoint over, and SO_EXCLUSIVEADDRUSE — which the default sets and my first version dropped — is what keeps the port ours. Applying the POSIX mapping there would have handed the port away, which is precisely the hazard reuse_port: false exists to avoid. Options is documented as POSIX only.

Known limitation, documented in the header

stop() releases the socket through cpp-httplib's Server::stop(), which only closes it if (is_running_). ~Server() is = default and never closes it. So a server that was bound but never listened keeps its port until the process ends. Enforcing the pairing here would mean either owning a thread inside the library or patching upstream; for now the header says so plainly.

Follow-up this opens

OpenDocument.droid can delete its choosePort/bindable probe and simply ask for port 0.

Callers and verification

CLI binds before printing its urls, prints the port it got, and owns its cache directory. The jni and python bindings gain bind() and Options, lose serveFile/cachePath, and both test suites drop the free-port dances they needed to work around the old API.

• odr_test --gtest_filter='HttpServer.*' — 4 new tests, all pass (new file; there was no C++ coverage of HttpServer before)
• jni JUnit HttpServerTest — 2 new tests pass locally, serveFile skipped for missing test data
• python test_http_server.py — 4 pass against a locally built pyodr_core, including the rewritten serve_file test that now translates and connects for itself
• cli/server serves a real odt end to end
• scripts/format and black --check clean

One detail worth knowing for the tests: localhost resolves to both ::1 and 127.0.0.1, so a "bind a port already in use" test against localhost quietly succeeds on the other address. The tests use literal 127.0.0.1.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fe3aaad83e

ℹ️ 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".