fix(database): keep the legacy tag cleanup from colliding with an existing tag (#6592)

* fix(database): avoid legacy inbound tag cleanup collisions

* test(database): assert the legacy tag cleanup keeps the migration green

The collision guard's test asserted only that the colliding tag was left
alone, which an unguarded cleanup also produces: the UPDATE fails on the
unique index and the row is unchanged either way. The cleanup shares a
transaction with every other requirement, so that failure rolls all of
them back on every boot and only reaches the log. Assert the call itself
succeeds, which is what actually distinguishes the two.

---------

Co-authored-by: n0ctal <n0ctal@users.noreply.github.com>
This commit is contained in:
n0ctal
2026-09-26 23:30:25 +05:00
committed by GitHub
parent c0c0136037
commit 3b9ca47a4e
2 changed files with 69 additions and 7 deletions
+13 -3
View File
@@ -263,11 +263,21 @@ func (s *InboundService) MigrationRequirements() (err error) {
// Must be cross-DB: INSTR/REPLACE work on SQLite; Postgres needs position().
tagCleanup := `UPDATE inbounds
SET tag = REPLACE(tag, '0.0.0.0:', '')
WHERE INSTR(tag, '0.0.0.0:') > 0;`
WHERE INSTR(tag, '0.0.0.0:') > 0
AND NOT EXISTS (
SELECT 1 FROM inbounds AS other
WHERE other.id <> inbounds.id
AND other.tag = REPLACE(inbounds.tag, '0.0.0.0:', '')
);`
if database.IsPostgres() {
tagCleanup = `UPDATE inbounds
tagCleanup = `UPDATE inbounds AS i
SET tag = REPLACE(tag, '0.0.0.0:', '')
WHERE position('0.0.0.0:' in tag) > 0;`
WHERE position('0.0.0.0:' in tag) > 0
AND NOT EXISTS (
SELECT 1 FROM inbounds AS other
WHERE other.id <> i.id
AND other.tag = REPLACE(i.tag, '0.0.0.0:', '')
);`
}
err = tx.Exec(tagCleanup).Error
if err != nil {
+56 -4
View File
@@ -144,9 +144,9 @@ func TestMigrationRequirements_CleansLegacyZeroAddrTag(t *testing.T) {
db := database.GetDB()
legacy := &model.Inbound{
UserId: 1,
Tag: "inbound-0.0.0.0:30002",
Tag: "inbound-0.0.0.0:30003",
Enable: true,
Port: 30002,
Port: 30003,
Protocol: model.VLESS,
Settings: `{"clients":[]}`,
StreamSettings: `{"security":"tls","tlsSettings":{"settings":{"domains":[{"domain":"example.com"}]}}}`,
@@ -162,8 +162,60 @@ func TestMigrationRequirements_CleansLegacyZeroAddrTag(t *testing.T) {
if err := db.First(&got, legacy.Id).Error; err != nil {
t.Fatalf("reload inbound: %v", err)
}
if got.Tag != "inbound-30002" {
t.Fatalf("legacy 0.0.0.0: tag not stripped: got %q, want %q", got.Tag, "inbound-30002")
if got.Tag != "inbound-30003" {
t.Fatalf("legacy 0.0.0.0: tag not stripped: got %q, want %q", got.Tag, "inbound-30003")
}
}
func TestMigrationRequirements_SkipsLegacyZeroAddrTagCollision(t *testing.T) {
setupConflictDB(t)
db := database.GetDB()
existing := &model.Inbound{
UserId: 1,
Tag: "inbound-30004",
Enable: true,
Port: 30004,
Protocol: model.VLESS,
Settings: `{"clients":[]}`,
StreamSettings: `{"network":"tcp","security":"none"}`,
}
legacy := &model.Inbound{
UserId: 1,
Tag: "inbound-0.0.0.0:30004",
Enable: true,
Port: 30005,
Protocol: model.VLESS,
Settings: `{"clients":[]}`,
StreamSettings: `{"security":"tls","tlsSettings":{"settings":{"domains":[{"domain":"example.com"}]}}}`,
}
if err := db.Create(existing).Error; err != nil {
t.Fatalf("create existing inbound: %v", err)
}
if err := db.Create(legacy).Error; err != nil {
t.Fatalf("create legacy inbound: %v", err)
}
svc := InboundService{}
// The cleanup shares a transaction with every other requirement, so a unique
// violation here rolls all of them back and only reaches the log.
if err := svc.MigrationRequirements(); err != nil {
t.Fatalf("MigrationRequirements: %v", err)
}
var got model.Inbound
if err := db.First(&got, legacy.Id).Error; err != nil {
t.Fatalf("reload legacy inbound: %v", err)
}
if got.Tag != "inbound-0.0.0.0:30004" {
t.Fatalf("colliding legacy tag should be left unchanged, got %q", got.Tag)
}
var count int64
if err := db.Model(&model.Inbound{}).Where("tag = ?", "inbound-30004").Count(&count).Error; err != nil {
t.Fatalf("count existing tag: %v", err)
}
if count != 1 {
t.Fatalf("target tag count = %d, want 1", count)
}
}