fix: support comma-separated bind addresses in host setting#1606
Conversation
|
Thanks for fixing this regression. The multi-socket uvicorn approach looks right to me, but I found a couple of things that need cleanup before I can merge it. The PR currently conflicts with main. Also, Could you rebase on current main and cover these cases with focused tests? |
Fixes the regression introduced when jundot#1526 switched from uvicorn.run() to uvicorn.Config + bind_socket() + Server.run(sockets=[...]): the old approach silently accepted a list of hosts via asyncio.create_server(), but bind_socket() calls sock.bind((host, port)) which requires a string. Changes: - settings.py: normalise legacy YAML list values (host: [a, b]) to a comma-separated string on load so the web UI and server both see a str - cli.py (serve_command): parse comma-separated hosts, bind one socket per address before model preload (preserving the fail-fast guarantee from jundot#1526), then pass all sockets to Server.run() - cli.py (launch_command): pick the first bind address as the connect host, normalising wildcards (0.0.0.0, ::) to 127.0.0.1 - utils/network.py: add is_valid_bind_host() which accepts 0.0.0.0 and :: unlike is_valid_alias() which rejects unspecified addresses - admin/routes.py: validate each comma-separated part on save, returning a readable 400 before an invalid value can crash the server on restart - admin/static/js/dashboard.js: extract .msg from Pydantic error objects instead of joining raw objects (fixes [object Object] display), two sites - admin/i18n/{en,zh,ja,ko,zh-TW}.json: add host_multi_hint string and drop "(caution)" from host_custom label now that multi-address is valid - admin/templates/dashboard/_settings.html: show host_multi_hint hint below the custom host input when the custom mode is active Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The caution warning is still valid — manually entering a bind address can inadvertently expose the server, and supporting multiple addresses makes that risk more rather than less relevant.
…osts is_valid_bind_host(): add a digit-dot regex guard so strings like 999.999.999.999 that fail ipaddress.ip_address() are not silently accepted as hostnames (digit-only dotted labels satisfy RFC 1123). detect_server_aliases(): split the host parameter on commas before checking for loopback/wildcard addresses, so values like "127.0.0.1, ::1" no longer drop localhost from the alias list. Adds focused tests covering valid IPv4/IPv6 (including unspecified addresses and IPv4-mapped IPv6), valid hostnames, IP-shaped-but-invalid values of various arity, malformed hostnames, and wrong types. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tted names is_valid_hostname(): require the rightmost label of any dotted name to contain at least one letter. IANA has never delegated a numeric TLD, so an all-digit last label (e.g. "999" in "999.999.999.999") indicates an IP-shaped string, not a real hostname. This mirrors the approach used by the validators PyPI library (python-validators), whose domain regex ends with [a-z] to enforce the same constraint. Single-label names (no dots) are local hostnames and are exempt from the TLD constraint. The IP-shaped fast-path guard in is_valid_bind_host() is retained as a cheap short-circuit, but the root fix is now in is_valid_hostname() so both is_valid_bind_host() and is_valid_alias() benefit. Adds is_valid_hostname tests for the new behaviour, and fixes test_accepts_hostname_with_dashes_instead_of_dots to verify that single-label names without letters (e.g. "192-168-1-1") are still accepted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@jundot — rebased on current main and addressed all three points from your review.
Added a fast-path guard that rejects strings like
Replaced the single-value Tests Added focused tests covering: valid IPv4/IPv6 (including unspecified addresses and IPv4-mapped IPv6), valid hostnames, IP-shaped-but-invalid values of various octet counts, the new |
|
Thanks for rebasing and tightening this. The multi-socket bind path and alias handling look good to me now, and I'm going to merge it. I noticed one small i18n consistency issue: |
Problem
#1526 introduced a pre-bind approach (
uvicorn.Config + bind_socket()) to fail fast on port conflicts before model preload. This broke multi-address binding:bind_socket()callssock.bind((host, port))which requires a plain string, not a list — so users who hadhost: [127.0.0.1, 192.168.1.x]in their settings (or set a comma-separated value) now get aTypeErroron startup.Supersedes #908, which patched the old
uvicorn.run()call that no longer exists.Fix
omlx/settings.py— normalise legacy YAML list values (host: [a, b]) to a comma-separated string on load. Backward-compatible: single-string configs are unchanged.omlx/cli.py(serve_command) — split the host string by comma, bind one socket per address (preserving the fail-fast guarantee from #1526), then pass all sockets toServer.run(sockets=[...]). uvicorn accepts a socket list and listens on all of them.omlx/cli.py(launch_command) — pick the first bind address as the connect host; normalise wildcards (0.0.0.0,::) to127.0.0.1.omlx/utils/network.py— addis_valid_bind_host(): accepts all valid IPs including0.0.0.0and::, unlikeis_valid_alias()which rejects unspecified addresses.omlx/admin/routes.py— validate each comma-separated part on save, returning a readable 400 before an invalid value can crash the server on restart.omlx/admin/static/js/dashboard.js— extract.msgfrom Pydantic error objects instead of joining raw objects (fixes[object Object]display). Two call sites.omlx/admin/i18n/{en,zh,ja,ko,zh-TW}.json— addhost_multi_hintstring in all five languages; drop "(caution)" fromhost_customlabel now that entering multiple addresses is a valid normal use case.omlx/admin/templates/dashboard/_settings.html— show thehost_multi_hinthint below the custom host input when custom mode is active.Usage
Enter multiple bind addresses separated by commas in the Host field:
Existing single-address settings and legacy YAML list values both continue to work without migration.
Testing
127.0.0.1, ::1in the web UI and save — both addresses should appear in startup output"host": ["127.0.0.1", "192.168.1.x"]— should load cleanly and show both bind linesnot a host!!) — should get a readable 400 error, not[object Object]omlx launchwith a multi-address host connects via the first address🤖 Generated with Claude Code