Files
3x-ui/sub/subService_test.go
Sanaei 19e88c4610 fix: address open bug reports (#4539, #4538, #4535, #4531, #4515) (#4545)
* fix: hash-storage panic on SIGHUP and seeder dup-key on cold restart (#4539)

Two bugs that combine into an unrecoverable crash loop after a user
enables the Telegram bot in settings on a fresh install.

1. CheckHashStorageJob.Run panics with a nil pointer dereference. The
   cron job is scheduled whenever settings say the bot is enabled, but
   the package-level hash storage is only initialized inside
   Tgbot.Start, which StartPanelOnly intentionally skips
   (startTgBot=false). Toggling the bot on via the panel triggers
   SIGHUP, the storage stays nil, and the cron fires 2 minutes later
   and panics, exiting 2.

2. seedClientsFromInboundJSON is not idempotent. The fresh-install
   early-return path recorded only UserPasswordHash + ApiTokensTable,
   never ClientsTable. After the admin adds clients via the panel
   (which writes to the clients table through SyncInbound), the next
   start runs the seeder for the first time, finds matching emails
   already in the table, and fails with SQLSTATE 23505 on
   idx_clients_email, turning the panic above into an unrecoverable
   crash loop on PostgreSQL.

Fixes:
- web/job/check_hash_storage.go: nil-check the storage before calling
  RemoveExpiredHashes.
- database/db.go: in the fresh-install early-return path, also record
  ClientsTable so the seeder never re-runs against panel-added data.
- database/db.go: hydrate seedClientsFromInboundJSON's byEmail cache
  from existing rows so it merges instead of inserting when a row with
  the same email already lives in the clients table.

Regression tests cover both paths.

Closes #4539

* fix(clients): preserve protocol-specific credentials across multi-inbound syncs (#4538)

fillProtocolDefaults only populates the credential relevant to the
inbound's protocol (c.ID for VLESS, c.Auth for Hysteria, c.Password
for Trojan/Shadowsocks). Each inbound's settings.clients JSON
therefore carries the same client with only one of those fields set.

SyncInbound's update path was unconditionally copying every credential
column from incoming to the existing clients row, so the second sync
(e.g. Hysteria after VLESS) would write UUID="" over a valid VLESS
UUID and Auth="" the other way around. The next GetXrayConfig then
emitted VLESS client entries with no "id" field, and xray-core
crashed on startup with "common/uuid: invalid UUID:".

Guard UUID/Password/Auth/Flow/Security/Reverse against empty
overwrites so each protocol's sync only writes the credentials it
actually owns. Other fields (LimitIP, TotalGB, Comment, etc.) keep
the existing copy-everything behavior so admins can still clear them
through the panel.

Regression test in client_sync_multiprotocol_test.go.

Closes #4538

* fix(expiry): show delayed-start countdown in subscribe and client info (#4535)

A client with "start after first use" expiry stores the duration as a
negative number of milliseconds (e.g. -86400000 = 1 day after first
connect). The clients page row already renders this correctly as
"Delayed start: 1d", but two other surfaces treated negative values as
zero and rendered them as unlimited:

- Subscription header: the index==0 / index>0 branches in subService,
  subClashService and subJsonService only carried ExpiryTime forward
  when > 0, so traffic.ExpiryTime stayed at zero and the header sent
  expire=0. Every imported client appeared to have no expiry, and the
  built-in subscribe page rendered the "unlimited" tag.

- ClientInfoModal: both the expiryLabel helper and the rendering check
  treated <= 0 as the "no expiry" branch, so the modal showed an
  infinity tag instead of "Delayed start: Nd".

Add subscriptionExpiryFromClient to map negative durations onto a
"now + |value|" timestamp so subscription clients see an actual expiry
they can count down from. Update ClientInfoModal's helper and render
to match the clients-page convention.

Regression test in subService_test.go covers the helper.

Refs #4535

* feat(clash): emit xhttp and httpupgrade transports in subscription (#4531)

applyTransport's switch only covered tcp/ws/grpc; xhttp and
httpupgrade inbounds fell through to the default branch and returned
false. buildProxy then returned a nil map and the inbound was dropped
from the Clash subscription. When the subscription only contained
xhttp/httpupgrade inbounds, the proxies list ended up empty and the
client saw a 404 (or an "Error!" body on older builds), then refused
to parse.

Add a case for each, mapping the inbound's stream settings onto the
Mihomo-format opts blocks:

  xhttp        -> xhttp-opts: { path, host, mode }
  httpupgrade  -> http-upgrade-opts: { path, headers: { Host } }

Host falls back to the headers map when the dedicated `host` field is
empty, matching the existing ws behavior.

Closes #4531

* fix(online): refresh online-clients list even when no WS frontend is connected (#4515)

XrayTrafficJob and NodeTrafficSyncJob both gated the entire
post-traffic-write block behind websocket.HasClients() to skip
expensive broadcasts when no browser is open. The block included the
RefreshOnlineClientsFromMap call that keeps the in-memory
p.onlineClients list current.

Several non-WS consumers read that same list:
- Telegram bot (tgbot.go calls p.GetOnlineClients in 3 places)
- REST GET /panel/api/onlines (returned to API callers)
- Internal alerts that check whether a client is online

When no browser was watching the dashboard, the list went stale and
stayed empty, so the bot reported "nobody online" and the onlines API
returned [] even when xray had active sessions.

Move RefreshOnlineClientsFromMap above the HasClients guard so the
in-memory list is always fresh. Only the actual BroadcastTraffic /
BroadcastClientStats / BroadcastOutbounds calls (and the
GetAllClientTraffics / GetInboundsTrafficSummary work that feeds them)
remain gated by HasClients.

Closes #4515

* fix: address copilot review on #4545

Two issues raised by the Copilot review:

1) subscriptionExpiryFromClient called time.Now() per invocation.
   Two clients with the same delayed-start duration normalized to
   timestamps a few milliseconds apart, so the aggregator's
   "if normalized != traffic.ExpiryTime" check tripped and the
   subscription header expire= dropped back to 0 — the exact bug
   the helper was meant to fix, just one client later.

   Take nowMs as a parameter; each of GetSubs / GetClash / GetConfig
   captures one timestamp per request and reuses it.

2) Guarding Flow against empty incoming values in SyncInbound
   prevented a user from ever clearing a VLESS flow via the panel.
   FlowOverride on client_inbounds is the per-inbound mechanism that
   already preserves flow correctly across protocols, so the guard
   on the shared clients.flow column is the wrong place.

   Drop the Flow guard, keep the rest (UUID/Password/Auth/Security/
   Reverse — none of which have a per-inbound override column).
   Adds a regression test that asserts clearing flow on the owning
   inbound makes ListForInbound return flow="".

   The existing cross-protocol test is rewritten to assert on the
   user-visible behavior (ListForInbound flow) instead of the shared
   clients.flow column.
2026-05-25 00:08:06 +02:00

18 KiB