Fix node id collision, host normalization and blank error text
Three defects made the panel look broken to the operator:
- POST /api/proxies with a Chinese name slugged the id down to its ASCII
digits ("香港01" -> "01"), collided with an existing node and returned
HTTP 500 "proxy id already exists". Non-ASCII names now get sequential
node-N ids; ASCII names keep a readable slug. Both paths de-duplicate.
- A host pasted as a URL ("http://192.168.3.71") went straight to DNS, so
every check failed with "[Errno -2] Name does not resolve". normalize_host
accepts host, host:port, scheme://user:pass@host:port and [IPv6]:port, and
existing configs are healed at startup.
- str(asyncio.TimeoutError()) is empty, so a black-holed proxy showed a blank
error in the UI and looked like an internal bug. describe_exc always
produces a reason.
Also:
- Expected validation failures return 400 with an error message instead of
500 + traceback; the frontend surfaces every API error as a toast.
- PUT/DELETE on an unknown proxy id return 404 instead of silently creating
or reporting ok=false with 200.
- POST /api/check/<id> forces a check on disabled nodes instead of returning
ok=true without doing anything.
- Port range is validated (1..65535).
- Store.close() lets tests release SQLite handles.
- 33 unittest cases cover id generation, host normalization, error text,
startup migration and manual checks.
This commit is contained in:
@@ -41,10 +41,26 @@ def load_json(path: Path) -> dict[str, Any]:
|
||||
|
||||
|
||||
def save_json(path: Path, data: dict[str, Any]) -> None:
|
||||
"""Persist JSON config.
|
||||
|
||||
Prefer atomic replace, but fall back to in-place write when the target is a
|
||||
Docker/macOS bind-mounted file. Those mounts commonly reject ``os.replace``
|
||||
with ``OSError: [Errno 16] Resource busy``, which previously aborted
|
||||
``upsert_proxy`` before ``restart_proxy_task`` and left new nodes without a
|
||||
monitor loop.
|
||||
"""
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
tmp = path.with_suffix(path.suffix + ".tmp")
|
||||
tmp.write_text(json.dumps(data, ensure_ascii=False, indent=2) + "\n", encoding="utf-8")
|
||||
tmp.replace(path)
|
||||
text = json.dumps(data, ensure_ascii=False, indent=2) + "\n"
|
||||
tmp = path.with_name(path.name + ".tmp")
|
||||
try:
|
||||
tmp.write_text(text, encoding="utf-8")
|
||||
tmp.replace(path)
|
||||
except OSError:
|
||||
# Bind-mounted single files cannot always be renamed into place.
|
||||
path.write_text(text, encoding="utf-8")
|
||||
with contextlib.suppress(Exception):
|
||||
if tmp.exists():
|
||||
tmp.unlink()
|
||||
|
||||
|
||||
def deep_copy(obj: Any) -> Any:
|
||||
@@ -145,6 +161,11 @@ class Store:
|
||||
self.lock = asyncio.Lock()
|
||||
self.init()
|
||||
|
||||
def close(self) -> None:
|
||||
"""Release the SQLite handle (used by tests to avoid ResourceWarning)."""
|
||||
with contextlib.suppress(Exception):
|
||||
self.conn.close()
|
||||
|
||||
def init(self) -> None:
|
||||
self.conn.executescript(
|
||||
"""
|
||||
@@ -461,6 +482,57 @@ async def get_exit_ip_via_socks(proxy: dict[str, Any], timeout: float) -> str |
|
||||
return None
|
||||
|
||||
|
||||
def normalize_host(raw: str) -> tuple[str, int | None]:
|
||||
"""Split a user-entered host field into (hostname, optional port).
|
||||
|
||||
Accepts bare hosts, ``host:port``, and full URLs such as
|
||||
``socks5://user@1.2.3.4:1080`` / ``http://192.168.3.71:7890``. Without this,
|
||||
a pasted URL is handed straight to DNS and every check fails with
|
||||
``[Errno -2] Name does not resolve``, which looks like a panel bug.
|
||||
"""
|
||||
s = (raw or "").strip().strip("/")
|
||||
if not s:
|
||||
return "", None
|
||||
if "://" in s:
|
||||
s = s.split("://", 1)[1]
|
||||
s = s.split("/", 1)[0]
|
||||
if "@" in s:
|
||||
s = s.rsplit("@", 1)[1]
|
||||
port: int | None = None
|
||||
if s.startswith("["): # bracketed IPv6, optionally with :port
|
||||
host, _, rest = s[1:].partition("]")
|
||||
if rest.startswith(":") and rest[1:].isdigit():
|
||||
port = int(rest[1:])
|
||||
return host, port
|
||||
if s.count(":") == 1:
|
||||
host, _, pstr = s.partition(":")
|
||||
if pstr.isdigit():
|
||||
return host, int(pstr)
|
||||
return host, None
|
||||
return s, None
|
||||
|
||||
|
||||
def describe_exc(e: BaseException) -> str:
|
||||
"""Human-readable failure reason.
|
||||
|
||||
``str(e)`` is empty for several exceptions that matter here -- most notably
|
||||
``asyncio.TimeoutError``/``TimeoutError``, which is what a firewalled or
|
||||
black-holed proxy endpoint produces. An empty error string in the panel
|
||||
looked like an internal bug, so always fall back to the class name and
|
||||
annotate the common cases.
|
||||
"""
|
||||
msg = str(e).strip()
|
||||
if isinstance(e, (asyncio.TimeoutError, TimeoutError)):
|
||||
return f"超时无响应:TCP 已连接但代理未回应握手({msg or 'timeout'})"
|
||||
if isinstance(e, asyncio.IncompleteReadError):
|
||||
return f"握手被中断:{msg or 'connection closed during handshake'}"
|
||||
if isinstance(e, ConnectionRefusedError):
|
||||
return "连接被拒绝:端口未监听"
|
||||
if isinstance(e, socket.gaierror):
|
||||
return f"域名解析失败:{msg or e}"
|
||||
return msg or e.__class__.__name__
|
||||
|
||||
|
||||
async def check_proxy(proxy: dict[str, Any]) -> tuple[bool, int | None, str | None, str | None]:
|
||||
timeout = float(proxy.get("timeout_seconds", 8))
|
||||
start = time.perf_counter()
|
||||
@@ -471,13 +543,14 @@ async def check_proxy(proxy: dict[str, Any]) -> tuple[bool, int | None, str | No
|
||||
err = None if exit_ip else (proxy.get("last_exit_ip_error") or "exit ip lookup failed")
|
||||
return True, latency, err, exit_ip
|
||||
except Exception as e:
|
||||
return False, int((time.perf_counter() - start) * 1000), str(e), None
|
||||
return False, int((time.perf_counter() - start) * 1000), describe_exc(e), None
|
||||
|
||||
|
||||
class App:
|
||||
def __init__(self, config_path: Path):
|
||||
self.config_path = config_path
|
||||
self.config = merge_defaults(load_json(config_path))
|
||||
self.migrate_proxy_hosts()
|
||||
if not config_path.exists(): save_json(config_path, self.config)
|
||||
self.store = Store(Path(self.config.get("db_path") or DEFAULT_DB))
|
||||
self.notifier = Notifier(self.config.get("notifications") or {}, self.store)
|
||||
@@ -493,6 +566,25 @@ class App:
|
||||
def channels(self) -> list[dict[str, Any]]:
|
||||
return self.config.setdefault("notifications", {}).setdefault("channels", [])
|
||||
|
||||
def migrate_proxy_hosts(self) -> None:
|
||||
"""Clean up hosts stored before ``normalize_host`` existed.
|
||||
|
||||
Nodes saved with a pasted URL (e.g. ``http://192.168.3.71``) fail every
|
||||
check with ``[Errno -2] Name does not resolve``. Rewrite them in place at
|
||||
startup so existing configs heal themselves; ``start_tasks`` persists the
|
||||
result.
|
||||
"""
|
||||
self._migrated = False
|
||||
for p in self.config.get("proxies") or []:
|
||||
raw = str(p.get("host") or "")
|
||||
host, url_port = normalize_host(raw)
|
||||
if host and host != raw:
|
||||
p["host"] = host
|
||||
if url_port and int(p.get("port") or 0) in (0, 1080):
|
||||
p["port"] = url_port
|
||||
self._migrated = True
|
||||
print(f"[migrate] proxy {p.get('id')}: host {raw!r} -> {host!r}:{p.get('port')}", flush=True)
|
||||
|
||||
def state_for(self, proxy_id: str) -> RuntimeState:
|
||||
return self.states.setdefault(proxy_id, RuntimeState())
|
||||
|
||||
@@ -507,13 +599,53 @@ class App:
|
||||
def get_channel(self, channel_id: str) -> dict[str, Any] | None:
|
||||
return next((c for c in self.channels if c.get("id") == channel_id), None)
|
||||
|
||||
def slug_id(self, name: str | None) -> str:
|
||||
"""Build a URL-safe unique node id from a (possibly non-ASCII) name.
|
||||
|
||||
Non-ASCII names (Chinese, emoji flags, ...) collapse to an empty or
|
||||
misleading slug -- "香港01" used to become "01", colliding with an
|
||||
existing node and making the POST fail with 500 "proxy id already
|
||||
exists". Keep the readable slug only for plain ASCII names; anything
|
||||
containing non-ASCII characters gets a sequential ``node-N`` id.
|
||||
Both paths are de-duplicated instead of raising.
|
||||
"""
|
||||
raw = (name or "").strip()
|
||||
base = ""
|
||||
if raw and re.fullmatch(r"[\x20-\x7e]+", raw):
|
||||
base = re.sub(r"[^a-zA-Z0-9_-]+", "-", raw).strip("-")
|
||||
if not base:
|
||||
for n in range(1, 10000):
|
||||
cand = f"node-{n}"
|
||||
if not self.get_proxy(cand):
|
||||
return cand
|
||||
return f"node-{uuid.uuid4().hex[:8]}"
|
||||
if not self.get_proxy(base):
|
||||
return base
|
||||
for n in range(2, 1000):
|
||||
cand = f"{base}-{n}"
|
||||
if not self.get_proxy(cand):
|
||||
return cand
|
||||
return f"{base}-{uuid.uuid4().hex[:8]}"
|
||||
|
||||
def validate_proxy(self, data: dict[str, Any], existing: dict[str, Any] | None = None) -> dict[str, Any]:
|
||||
p = dict(existing or {})
|
||||
p.update(data)
|
||||
p.setdefault("id", re.sub(r"[^a-zA-Z0-9_-]+", "-", (p.get("name") or "proxy").strip()).strip("-") or f"proxy-{uuid.uuid4().hex[:8]}")
|
||||
if not p.get("id"):
|
||||
p["id"] = self.slug_id(p.get("name"))
|
||||
p.setdefault("name", p["id"])
|
||||
if not p.get("host"): raise ValueError("host required")
|
||||
p["port"] = int(p.get("port") or 1080)
|
||||
# Users paste "http://1.2.3.4:7890" or "1.2.3.4:7890" into the Host
|
||||
# field; the raw string then fails DNS with "Name does not resolve".
|
||||
host, url_port = normalize_host(str(p["host"]))
|
||||
if not host:
|
||||
raise ValueError("host required")
|
||||
p["host"] = host
|
||||
# A port embedded in the pasted host wins over an untouched default.
|
||||
if url_port and int(p.get("port") or 0) in (0, 1080):
|
||||
p["port"] = url_port
|
||||
p["port"] = int(p.get("port") or url_port or 1080)
|
||||
if not 1 <= p["port"] <= 65535:
|
||||
raise ValueError("port must be 1..65535")
|
||||
p["target_host"] = p.get("target_host") or self.config.get("target_host") or "www.baidu.com"
|
||||
p["target_port"] = int(p.get("target_port") or self.config.get("target_port") or 443)
|
||||
p["timeout_seconds"] = int(p.get("timeout_seconds") or self.config.get("timeout_seconds") or 8)
|
||||
@@ -587,7 +719,16 @@ class App:
|
||||
else:
|
||||
if self.get_proxy(p["id"]): raise ValueError("proxy id already exists")
|
||||
self.proxies.append(p)
|
||||
await self.save_config(); await self.restart_proxy_task(p["id"])
|
||||
# Always (re)start the monitor loop after the in-memory update so a
|
||||
# transient config-write failure cannot leave a node without polling.
|
||||
save_error: Exception | None = None
|
||||
try:
|
||||
await self.save_config()
|
||||
except Exception as exc: # noqa: BLE001 - re-raise after starting monitor
|
||||
save_error = exc
|
||||
await self.restart_proxy_task(p["id"])
|
||||
if save_error is not None:
|
||||
raise save_error
|
||||
return public_proxy(p)
|
||||
|
||||
async def delete_proxy(self, proxy_id: str) -> bool:
|
||||
@@ -618,8 +759,13 @@ class App:
|
||||
await self.save_config()
|
||||
return True
|
||||
|
||||
async def run_once(self, proxy: dict[str, Any]) -> None:
|
||||
if not proxy.get("enabled", True): return
|
||||
async def run_once(self, proxy: dict[str, Any], force: bool = False) -> None:
|
||||
"""Run one check. ``force`` bypasses the enabled flag for manual checks.
|
||||
|
||||
Without ``force`` a manual "check now" on a disabled node returned
|
||||
``{"ok": true}`` while silently doing nothing.
|
||||
"""
|
||||
if not force and not proxy.get("enabled", True): return
|
||||
proxy_id = proxy["id"]
|
||||
st = self.state_for(proxy_id)
|
||||
ok, latency, err, exit_ip = await check_proxy(proxy)
|
||||
@@ -649,20 +795,34 @@ class App:
|
||||
st.notified_down = True
|
||||
|
||||
async def monitor_loop(self, proxy_id: str) -> None:
|
||||
while True:
|
||||
proxy = self.get_proxy(proxy_id)
|
||||
if not proxy: return
|
||||
if proxy.get("enabled", True):
|
||||
await self.run_once(proxy)
|
||||
await asyncio.sleep(int(proxy.get("interval_seconds", self.config.get("interval_seconds", 60))))
|
||||
try:
|
||||
while True:
|
||||
proxy = self.get_proxy(proxy_id)
|
||||
if not proxy:
|
||||
return
|
||||
if proxy.get("enabled", True):
|
||||
await self.run_once(proxy)
|
||||
interval = int(proxy.get("interval_seconds", self.config.get("interval_seconds", 60)))
|
||||
await asyncio.sleep(max(5, interval))
|
||||
except asyncio.CancelledError:
|
||||
raise
|
||||
|
||||
async def restart_proxy_task(self, proxy_id: str) -> None:
|
||||
old = self.monitor_tasks.pop(proxy_id, None)
|
||||
if old: old.cancel()
|
||||
if self.get_proxy(proxy_id):
|
||||
if old:
|
||||
old.cancel()
|
||||
with contextlib.suppress(asyncio.CancelledError, Exception):
|
||||
await old
|
||||
proxy = self.get_proxy(proxy_id)
|
||||
if proxy and proxy.get("enabled", True):
|
||||
self.monitor_tasks[proxy_id] = asyncio.create_task(self.monitor_loop(proxy_id))
|
||||
elif proxy and not proxy.get("enabled", True):
|
||||
# Keep disabled nodes registered without an active loop.
|
||||
self.monitor_tasks.pop(proxy_id, None)
|
||||
|
||||
async def start_tasks(self) -> None:
|
||||
if self._migrated:
|
||||
await self.save_config()
|
||||
for p in self.proxies:
|
||||
await self.restart_proxy_task(p["id"])
|
||||
|
||||
@@ -725,15 +885,21 @@ async def handle_http(reader: asyncio.StreamReader, writer: asyncio.StreamWriter
|
||||
status, ctype, resp = json_response({"ok": True, "proxy": await app.upsert_proxy(data)}, 201)
|
||||
elif method == "PUT" and p.startswith("/api/proxies/"):
|
||||
pid = urllib.parse.unquote(p.rsplit("/", 1)[-1])
|
||||
status, ctype, resp = json_response({"ok": True, "proxy": await app.upsert_proxy(data, pid)})
|
||||
if not app.get_proxy(pid):
|
||||
status, ctype, resp = json_response({"ok": False, "error": "proxy not found"}, 404)
|
||||
else:
|
||||
status, ctype, resp = json_response({"ok": True, "proxy": await app.upsert_proxy(data, pid)})
|
||||
elif method == "DELETE" and p.startswith("/api/proxies/"):
|
||||
pid = urllib.parse.unquote(p.rsplit("/", 1)[-1])
|
||||
status, ctype, resp = json_response({"ok": await app.delete_proxy(pid)})
|
||||
if await app.delete_proxy(pid):
|
||||
status, ctype, resp = json_response({"ok": True})
|
||||
else:
|
||||
status, ctype, resp = json_response({"ok": False, "error": "proxy not found"}, 404)
|
||||
elif method == "POST" and p.startswith("/api/check/"):
|
||||
pid = urllib.parse.unquote(p.rsplit("/", 1)[-1]); proxy = app.get_proxy(pid)
|
||||
if not proxy: status, ctype, resp = json_response({"ok": False, "error": "proxy not found"}, 404)
|
||||
else:
|
||||
await app.run_once(proxy)
|
||||
await app.run_once(proxy, force=True)
|
||||
status, ctype, resp = json_response({"ok": True, "state": dataclasses.asdict(app.state_for(pid))})
|
||||
elif method == "POST" and p == "/api/channels":
|
||||
status, ctype, resp = json_response({"ok": True, "channel": await app.upsert_channel(data)}, 201)
|
||||
@@ -752,9 +918,11 @@ async def handle_http(reader: asyncio.StreamReader, writer: asyncio.StreamWriter
|
||||
status, ctype, resp = json_response({"ok": True, "results": res})
|
||||
else:
|
||||
status, ctype, resp = json_response({"ok": False, "error": "not found"}, 404)
|
||||
except ValueError as e:
|
||||
status, ctype, resp = json_response({"ok": False, "error": str(e)}, 400)
|
||||
except Exception as e:
|
||||
traceback.print_exc()
|
||||
status, ctype, resp = json_response({"ok": False, "error": str(e)}, 500)
|
||||
status, ctype, resp = json_response({"ok": False, "error": describe_exc(e)}, 500)
|
||||
try:
|
||||
reason = {200:"OK",201:"Created",400:"Bad Request",404:"Not Found",500:"Internal Server Error"}.get(status,"OK")
|
||||
writer.write(f"HTTP/1.1 {status} {reason}\r\nContent-Type: {ctype}\r\nContent-Length: {len(resp)}\r\nConnection: close\r\n\r\n".encode()+resp)
|
||||
|
||||
Reference in New Issue
Block a user