diff --git a/internal/web/controller/inbound.go b/internal/web/controller/inbound.go index 297470f..2d93101 100644 --- a/internal/web/controller/inbound.go +++ b/internal/web/controller/inbound.go @@ -4,7 +4,9 @@ import ( "bytes" "encoding/json" "io" + "mime" "net" + "net/url" "strconv" "strings" @@ -224,11 +226,17 @@ func (a *InboundController) bulkDelInbounds(c *gin.Context) { notifyClientsChanged() } -// requestHasJSONKey reports whether the request body is a JSON object that -// explicitly carries key — present with a null value counts. The body is -// restored so the normal bind can read it again. Non-JSON bodies (form -// posts) report false for every key. -func requestHasJSONKey(c *gin.Context, key string) bool { +// requestHasKey reports whether the request body explicitly carries key — +// present with a JSON null or an empty form value counts. The probe must +// understand both wire formats this endpoint receives: API clients post +// JSON, but the panel frontend posts application/x-www-form-urlencoded by +// default (frontend/src/api/http-init.ts form-encodes every POST that does +// not declare a JSON Content-Type, encoding a null value as a bare "key="). +// A JSON-only probe silently reports false for every real browser post — +// that is exactly how browser-driven nodeId reassignments became a no-op +// while JSON callers kept working. The body is restored so the normal +// bind can read it again. +func requestHasKey(c *gin.Context, key string) bool { if c.Request == nil || c.Request.Body == nil { return false } @@ -237,11 +245,27 @@ func requestHasJSONKey(c *gin.Context, key string) bool { return false } c.Request.Body = io.NopCloser(bytes.NewReader(body)) - var probe map[string]json.RawMessage - if json.Unmarshal(body, &probe) != nil { - return false + + mediaType, _, _ := mime.ParseMediaType(c.GetHeader("Content-Type")) + if mediaType == "application/x-www-form-urlencoded" { + return formHasKey(body, key) } - _, ok := probe[key] + var probe map[string]json.RawMessage + if json.Unmarshal(body, &probe) == nil { + _, ok := probe[key] + return ok + } + // Undeclared or unexpected Content-Type with a non-JSON body: fall + // back to form parsing so the key is still understood. + return formHasKey(body, key) +} + +// formHasKey reports whether a urlencoded body carries key, even with an +// empty value. ParseQuery keeps every valid pair it finds, so its map is +// usable even when it also returns an error for a later malformed pair. +func formHasKey(body []byte, key string) bool { + values, _ := url.ParseQuery(string(body)) + _, ok := values[key] return ok } @@ -255,16 +279,19 @@ func (a *InboundController) updateInbound(c *gin.Context) { inbound := &model.Inbound{ Id: id, } - // An explicitly sent nodeId (even null) is a (re)assignment request — - // UpdateInbound migrates the inbound when it differs from the stored - // assignment. An omitted nodeId keeps the stored assignment, so detect - // the key's presence on the raw body before binding consumes it. - nodeIDProvided := requestHasJSONKey(c, "nodeId") + // An explicitly sent nodeId (JSON null, or the form encoding of one — + // a bare "nodeId=") is a (re)assignment request: UpdateInbound migrates + // the inbound when it differs from the stored assignment. An omitted + // nodeId keeps the stored assignment, so detect the key's presence on + // the raw body before binding consumes it. + nodeIDProvided := requestHasKey(c, "nodeId") if !middleware.BindAndValidateInto(c, inbound) { return } inbound.NodeIDProvided = nodeIDProvided - // Same NodeID=0 → nil normalisation as addInbound. + // Same NodeID=0 → nil normalisation as addInbound. This also covers + // the browser's encoding of "back to local": the form post carries a + // bare "nodeId=", which gin binds as *int(0) rather than nil. if inbound.NodeID != nil && *inbound.NodeID == 0 { inbound.NodeID = nil } diff --git a/internal/web/controller/inbound_update_nodeid_wire_test.go b/internal/web/controller/inbound_update_nodeid_wire_test.go new file mode 100644 index 0000000..a9bc9a4 --- /dev/null +++ b/internal/web/controller/inbound_update_nodeid_wire_test.go @@ -0,0 +1,334 @@ +package controller + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "path/filepath" + "strconv" + "strings" + "testing" + + "github.com/gin-contrib/sessions" + "github.com/gin-contrib/sessions/cookie" + "github.com/gin-gonic/gin" + "github.com/op/go-logging" + + "github.com/mhsanaei/3x-ui/v3/internal/database" + "github.com/mhsanaei/3x-ui/v3/internal/database/model" + xuilogger "github.com/mhsanaei/3x-ui/v3/internal/logger" + "github.com/mhsanaei/3x-ui/v3/internal/web/service/panel" + "github.com/mhsanaei/3x-ui/v3/internal/web/session" + "github.com/mhsanaei/3x-ui/v3/internal/xray" +) + +// Regression tests for the P8.1 hotfix: browser-driven inbound +// re-homing (the "部署到" picker in the edit dialog) silently did nothing. +// +// Root cause: the panel frontend posts application/x-www-form-urlencoded +// by default (frontend/src/api/http-init.ts form-encodes every POST that +// does not declare a JSON Content-Type), but updateInbound detected an +// explicit nodeId with a JSON-only probe of the raw body. The probe +// reported false for every real browser post, so UpdateInbound kept the +// stored assignment. JSON API callers worked — including the P8 sandbox +// E2E — which is why these tests post the *form* wire format exactly as +// the browser builds it (the field set of formValuesToWirePayload in +// frontend/src/lib/xray/inbound-form-adapter.ts, encodeForm semantics: +// every key present, a null value emitted as a bare "key="). + +func newInboundWireEngine(t *testing.T) *gin.Engine { + t.Helper() + xuilogger.InitLogger(logging.ERROR) + gin.SetMode(gin.TestMode) + dbDir := t.TempDir() + t.Setenv("XUI_DB_FOLDER", dbDir) + if err := database.InitDB(filepath.Join(dbDir, "x-ui.db")); err != nil { + t.Fatalf("InitDB: %v", err) + } + t.Cleanup(func() { _ = database.CloseDB() }) + + engine := gin.New() + store := cookie.NewStore([]byte("inbound-wire-test-secret")) + engine.Use(sessions.Sessions("3x-ui", store)) + // Logs in as the first user so updateInbound's session lookup works, + // mirroring the production session path over a cookie round-trip. + engine.GET("/test-login", func(c *gin.Context) { + u, err := (&panel.UserService{}).GetFirstUser() + if err != nil { + c.Status(http.StatusInternalServerError) + return + } + if err := session.SetLoginUser(c, u); err != nil { + c.Status(http.StatusInternalServerError) + return + } + c.Status(http.StatusOK) + }) + NewInboundController(engine.Group("/panel/api/inbounds")) + return engine +} + +func inboundWireLogin(t *testing.T, engine *gin.Engine) []*http.Cookie { + t.Helper() + w := httptest.NewRecorder() + engine.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/test-login", nil)) + if w.Code != http.StatusOK { + t.Fatalf("test login: status %d", w.Code) + } + cookies := w.Result().Cookies() + if len(cookies) == 0 { + t.Fatal("test login: no session cookie issued") + } + return cookies +} + +func seedWireNode(t *testing.T, name string) *model.Node { + t.Helper() + n := &model.Node{ + Name: name, + Scheme: "http", + Address: "127.0.0.1", + Port: 1, // unreachable on purpose: UpdateInbound never dials nodes + BasePath: "/", + ApiToken: "tok", + Enable: true, + AllowPrivateAddress: true, + Status: "online", + InboundSyncMode: "all", + InboundsAdoptedAt: 1, + } + if err := database.GetDB().Create(n).Error; err != nil { + t.Fatalf("seed node %s: %v", name, err) + } + return n +} + +const wireTestSettings = `{"clients":[{"id":"11111111-2222-3333-4444-555555555555","email":"wire@example.com","enable":true}]}` + +func seedWireInbound(t *testing.T, tag string, port int, nodeID *int) model.Inbound { + t.Helper() + ib := &model.Inbound{ + Tag: tag, + Enable: true, + Port: port, + Protocol: model.VLESS, + Settings: wireTestSettings, + StreamSettings: `{"network":"tcp"}`, + Sniffing: `{"enabled":false}`, + NodeID: nodeID, + ShareAddrStrategy: "node", + TrafficReset: "never", + } + if err := database.GetDB().Create(ib).Error; err != nil { + t.Fatalf("seed inbound: %v", err) + } + ct := &xray.ClientTraffic{InboundId: ib.Id, Enable: true, Email: "wire@example.com", Up: 111, Down: 222} + if err := database.GetDB().Create(ct).Error; err != nil { + t.Fatalf("seed client traffic: %v", err) + } + return reloadWireInbound(t, ib.Id) +} + +func reloadWireInbound(t *testing.T, id int) model.Inbound { + t.Helper() + var ib model.Inbound + if err := database.GetDB().First(&ib, id).Error; err != nil { + t.Fatalf("reload inbound %d: %v", id, err) + } + return ib +} + +func wireNodeDirty(t *testing.T, id int) bool { + t.Helper() + var n model.Node + if err := database.GetDB().First(&n, id).Error; err != nil { + t.Fatalf("reload node %d: %v", id, err) + } + return n.ConfigDirty +} + +func wireTrafficIntact(t *testing.T) { + t.Helper() + var ct xray.ClientTraffic + if err := database.GetDB().Where("email = ?", "wire@example.com").First(&ct).Error; err != nil { + t.Fatalf("read client traffic: %v", err) + } + if ct.Up != 111 || ct.Down != 222 { + t.Fatalf("client traffic changed: up=%d down=%d, want 111/222", ct.Up, ct.Down) + } +} + +// browserFormBody renders the update body exactly as the browser sends +// it: the formValuesToWirePayload field set, url-encoded, with nodeId +// present as a number, present-but-empty (the null/local encoding), or +// omitted entirely (legacy callers), per the arguments. +func browserFormBody(ib model.Inbound, nodeID *int, omitNodeID bool) string { + v := url.Values{} + v.Set("up", strconv.FormatInt(ib.Up, 10)) + v.Set("down", strconv.FormatInt(ib.Down, 10)) + v.Set("total", strconv.FormatInt(ib.Total, 10)) + v.Set("remark", ib.Remark) + v.Set("enable", strconv.FormatBool(ib.Enable)) + v.Set("expiryTime", strconv.FormatInt(ib.ExpiryTime, 10)) + v.Set("trafficReset", ib.TrafficReset) + v.Set("trafficResetDay", strconv.Itoa(ib.TrafficResetDay)) + v.Set("lastTrafficResetTime", strconv.FormatInt(ib.LastTrafficResetTime, 10)) + v.Set("listen", ib.Listen) + v.Set("port", strconv.Itoa(ib.Port)) + v.Set("protocol", string(ib.Protocol)) + v.Set("settings", ib.Settings) + v.Set("streamSettings", ib.StreamSettings) + v.Set("sniffing", ib.Sniffing) + v.Set("tag", ib.Tag) + v.Set("shareAddrStrategy", ib.ShareAddrStrategy) + v.Set("shareAddr", ib.ShareAddr) + v.Set("subSortIndex", strconv.Itoa(ib.SubSortIndex)) + v.Set("disableFlow", strconv.FormatBool(ib.DisableFlow)) + if !omitNodeID { + if nodeID == nil { + v.Set("nodeId", "") // encodeForm renders a null as a bare "nodeId=" + } else { + v.Set("nodeId", strconv.Itoa(*nodeID)) + } + } + return v.Encode() +} + +type wireEnvelope struct { + Success bool `json:"success"` + Msg string `json:"msg"` +} + +func postInboundUpdate(t *testing.T, engine *gin.Engine, cookies []*http.Cookie, id int, body, contentType string) wireEnvelope { + t.Helper() + req := httptest.NewRequest(http.MethodPost, fmt.Sprintf("/panel/api/inbounds/update/%d", id), strings.NewReader(body)) + req.Header.Set("Content-Type", contentType) + for _, ck := range cookies { + req.AddCookie(ck) + } + w := httptest.NewRecorder() + engine.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("update %d: HTTP %d, body=%s", id, w.Code, w.Body.String()) + } + var env wireEnvelope + if err := json.Unmarshal(w.Body.Bytes(), &env); err != nil { + t.Fatalf("update %d: decode envelope: %v body=%s", id, err, w.Body.String()) + } + return env +} + +// (a) The browser's headline flow: a form post carrying nodeId= +// must perform the move — row reassigned, both nodes marked dirty, +// clients and traffic untouched. +func TestUpdateInboundFormWire_MoveBetweenNodes(t *testing.T) { + engine := newInboundWireEngine(t) + cookies := inboundWireLogin(t, engine) + nodeA := seedWireNode(t, "wire-a") + nodeB := seedWireNode(t, "wire-b") + seeded := seedWireInbound(t, "wire-move", 2461, &nodeA.Id) + + body := browserFormBody(seeded, &nodeB.Id, false) + env := postInboundUpdate(t, engine, cookies, seeded.Id, body, "application/x-www-form-urlencoded") + if !env.Success { + t.Fatalf("form move update failed: msg=%q", env.Msg) + } + + after := reloadWireInbound(t, seeded.Id) + if after.NodeID == nil || *after.NodeID != nodeB.Id { + t.Fatalf("node_id after form move = %v, want %d", after.NodeID, nodeB.Id) + } + if !wireNodeDirty(t, nodeA.Id) || !wireNodeDirty(t, nodeB.Id) { + t.Fatalf("both nodes must be dirty after a move (A=%v B=%v)", + wireNodeDirty(t, nodeA.Id), wireNodeDirty(t, nodeB.Id)) + } + wireTrafficIntact(t) +} + +// (b) A form post without nodeId keeps the stored assignment (the legacy +// caller contract, now exercised over the form wire format too). +func TestUpdateInboundFormWire_OmittedNodeIDKeepsAssignment(t *testing.T) { + engine := newInboundWireEngine(t) + cookies := inboundWireLogin(t, engine) + nodeA := seedWireNode(t, "wire-a") + seedWireNode(t, "wire-b") + seeded := seedWireInbound(t, "wire-keep", 2462, &nodeA.Id) + + body := browserFormBody(seeded, nil, true) + env := postInboundUpdate(t, engine, cookies, seeded.Id, body, "application/x-www-form-urlencoded") + if !env.Success { + t.Fatalf("form no-nodeId update failed: msg=%q", env.Msg) + } + + after := reloadWireInbound(t, seeded.Id) + if after.NodeID == nil || *after.NodeID != nodeA.Id { + t.Fatalf("node_id after omitted-nodeId form update = %v, want %d (unchanged)", after.NodeID, nodeA.Id) + } + wireTrafficIntact(t) +} + +// (c) The browser encodes "back to local panel" (allowClear on the +// picker) as a bare "nodeId=". That must count as provided with a nil +// target — and must not break binding of the *int field. +func TestUpdateInboundFormWire_EmptyNodeIDMovesToLocal(t *testing.T) { + engine := newInboundWireEngine(t) + cookies := inboundWireLogin(t, engine) + nodeA := seedWireNode(t, "wire-a") + seeded := seedWireInbound(t, "wire-local", 2463, &nodeA.Id) + + body := browserFormBody(seeded, nil, false) + if !strings.Contains(body, "nodeId=") { + t.Fatalf("test body must carry the bare nodeId= encoding, got %q", body) + } + env := postInboundUpdate(t, engine, cookies, seeded.Id, body, "application/x-www-form-urlencoded") + if !env.Success { + t.Fatalf("form empty-nodeId update failed: msg=%q", env.Msg) + } + + after := reloadWireInbound(t, seeded.Id) + if after.NodeID != nil { + t.Fatalf("node_id after empty-nodeId form update = %v, want nil (local)", after.NodeID) + } + if !wireNodeDirty(t, nodeA.Id) { + t.Fatal("source node must be dirty after a move to local") + } + wireTrafficIntact(t) +} + +// (d) The JSON caller path keeps working unchanged at the HTTP level. +func TestUpdateInboundJSONWire_MoveBetweenNodes(t *testing.T) { + engine := newInboundWireEngine(t) + cookies := inboundWireLogin(t, engine) + nodeA := seedWireNode(t, "wire-a") + nodeB := seedWireNode(t, "wire-b") + seeded := seedWireInbound(t, "wire-json", 2464, &nodeA.Id) + + payload := map[string]any{ + "up": seeded.Up, "down": seeded.Down, "total": seeded.Total, + "remark": seeded.Remark, "enable": seeded.Enable, + "expiryTime": seeded.ExpiryTime, "trafficReset": seeded.TrafficReset, + "trafficResetDay": seeded.TrafficResetDay, "lastTrafficResetTime": seeded.LastTrafficResetTime, + "listen": seeded.Listen, "port": seeded.Port, "protocol": string(seeded.Protocol), + "settings": seeded.Settings, "streamSettings": seeded.StreamSettings, + "sniffing": seeded.Sniffing, "tag": seeded.Tag, + "shareAddrStrategy": seeded.ShareAddrStrategy, "shareAddr": seeded.ShareAddr, + "subSortIndex": seeded.SubSortIndex, "disableFlow": seeded.DisableFlow, + "nodeId": nodeB.Id, + } + raw, err := json.Marshal(payload) + if err != nil { + t.Fatalf("marshal payload: %v", err) + } + env := postInboundUpdate(t, engine, cookies, seeded.Id, string(raw), "application/json") + if !env.Success { + t.Fatalf("JSON move update failed: msg=%q", env.Msg) + } + + after := reloadWireInbound(t, seeded.Id) + if after.NodeID == nil || *after.NodeID != nodeB.Id { + t.Fatalf("node_id after JSON move = %v, want %d", after.NodeID, nodeB.Id) + } + wireTrafficIntact(t) +}