diff --git a/common/cf-lock.c b/common/cf-lock.c index 24db4932..cc71ab5a 100644 --- a/common/cf-lock.c +++ b/common/cf-lock.c @@ -66,24 +66,24 @@ like_descendant_pattern (const char *path) return g_string_free (pattern, FALSE); } -/* Lock and resource share the Hub migration's UID and seafile-db. The SQL - * retains manual locks and suspended checkout reservations. - * A failed query refuses the write rather than weakening the barrier. */ +/* Use the current single V2 Guard table; the old cf_resource / + * lifecycle_ref/lease_until join is intentionally retired. Both C and Go + * deny ordinary writes to *any* active Checkout, including the owner's + * unscoped write. Controlled publication remains a separate proof path. */ static int load_exact (const char *repo_id, const char *path, const char *user) { + (void)user; char *hash = g_compute_checksum_for_string (G_CHECKSUM_SHA256, path, -1); - int n = 0; - if (n == 0) - n = seaf_db_statement_foreach_row ( - cf_ext_database (), - "SELECT r.uid FROM cf_resource r JOIN cf_edit_guard g ON g.resource_uid=r.uid " - "WHERE r.repo_id=? AND r.path_hash=? AND r.path=? AND r.kind='file' AND r.state='active' " - "AND g.repo_id=r.repo_id AND g.guard_id IS NOT NULL " - "AND (g.lifecycle_ref<>r.lifecycle_ref OR g.mode IS NULL OR g.mode<>'file-lock' OR g.owner_native_user IS NULL OR g.owner_native_user<>?) LIMIT 1", - barrier_row, NULL, 4, "string", repo_id, "string", hash, - "string", path, "string", user ? user : ""); - + if (!hash) + return -1; + int n = seaf_db_statement_foreach_row ( + cf_ext_database (), + "SELECT path FROM cf_edit_guard WHERE repo_id=? AND path_hash=UNHEX(?) " + "AND path=? AND guard_id IS NOT NULL " + "AND lease_expires_at>UTC_TIMESTAMP(6) LIMIT 1", + barrier_row, NULL, 3, "string", repo_id, "string", hash, + "string", path); g_free (hash); return n; } @@ -92,15 +92,14 @@ static int load_descendant (const char *repo_id, const char *path) { char *pattern = like_descendant_pattern (path); - int n = 0; - if (n == 0) - n = seaf_db_statement_foreach_row ( - cf_ext_database (), - "SELECT r.uid FROM cf_resource r JOIN cf_edit_guard g ON g.resource_uid=r.uid " - "WHERE r.repo_id=? AND r.path LIKE ? ESCAPE '\\\\' AND r.state='active' " - "AND g.repo_id=r.repo_id AND g.guard_id IS NOT NULL LIMIT 1", - barrier_row, NULL, 2, "string", repo_id, "string", pattern); - + if (!pattern) + return -1; + int n = seaf_db_statement_foreach_row ( + cf_ext_database (), + "SELECT path FROM cf_edit_guard WHERE repo_id=? " + "AND path LIKE ? ESCAPE '\\\\' AND guard_id IS NOT NULL " + "AND lease_expires_at>UTC_TIMESTAMP(6) LIMIT 1", + barrier_row, NULL, 2, "string", repo_id, "string", pattern); g_free (pattern); return n; } @@ -112,10 +111,15 @@ check_path (const char *repo_id, const char *path, const char *user, int n; if (!cf_ext_database ()) goto unavailable; + /* A legacy Guard schema cannot silently behave as an empty lock set. + * Test the required native index at the entry; the locked queries below + * also require current path and expiry columns. */ int ready = seaf_db_statement_foreach_row ( cf_ext_database (), - "SELECT version FROM cf_schema_migration WHERE version='029_editing_core' " - "AND state='applied' AND step=2", barrier_row, NULL, 0); + "SELECT index_name FROM information_schema.statistics " + "WHERE table_schema=DATABASE() AND table_name='cf_edit_guard' " + "AND index_name='cf_edit_guard_active' LIMIT 1", + barrier_row, NULL, 0); if (ready != 1) goto unavailable; n = load_exact (repo_id, path, user); @@ -176,7 +180,20 @@ void cf_lock_init (void) { char *backend = cf_ext_config_string ("lock_backend"); - lock_on = cf_ext_config_bool ("file_lock_enabled") && + gboolean installed = FALSE; + if (cf_ext_database () && seaf_db_type (cf_ext_database ()) == SEAF_DB_TYPE_MYSQL) { + int rows = seaf_db_statement_foreach_row ( + cf_ext_database (), + "SELECT table_name FROM information_schema.tables " + "WHERE table_schema=DATABASE() AND table_name='cf_edit_guard'", + barrier_row, NULL, 0); + /* An unreadable schema is not proof the table is absent. Preserve the + * write barrier and fail closed in check_path until it is repaired. */ + installed = rows != 0; + } + /* Checkout must protect native C writes even when the old optional + * file_lock_enabled setting was never configured. */ + lock_on = (installed || cf_ext_config_bool ("file_lock_enabled")) && (!backend || strcmp (backend, "cloudfile") == 0); if (backend && strcmp (backend, "cloudfile") != 0) seaf_warning ("CloudFile: unsupported lock backend '%s'.\n", backend); diff --git a/fileserver/cf_v2_edit_schema.go b/fileserver/cf_v2_edit_schema.go new file mode 100644 index 00000000..7265fa27 --- /dev/null +++ b/fileserver/cf_v2_edit_schema.go @@ -0,0 +1,130 @@ +package main + +// Validate the *installed* V2 Guard before accepting any file-server traffic. +// Older cf_edit_guard tables from the 029_editing_core migration use a +// resource_uid/lifecycle_ref/lease_until contract instead of the single V2 +// (repo_id,path_hash)/path/lease_expires_at contract. A table-name probe alone +// would enable the final Branch barrier and break all ordinary uploads. +import ( + "context" + "database/sql" + "fmt" + "strings" +) + +type cfGuardColumn struct { + kind string + length int64 + nullable string + unsigned bool +} +type cfGuardIndexPart struct { + name string + nonUnique int64 + prefix bool +} + +var cfV2GuardColumns = map[string]cfGuardColumn{ + "repo_id": {"char", 36, "NO", false}, + "path_hash": {"binary", 32, "NO", false}, + "path": {"varchar", 4096, "NO", false}, + "guard_id": {"char", 36, "YES", false}, + "generation": {"bigint", 0, "NO", true}, + "owner_uid": {"varchar", 225, "YES", false}, + "owner_native_user": {"varchar", 255, "YES", false}, + "terminal_type": {"varchar", 16, "YES", false}, + "terminal_name": {"varchar", 128, "YES", false}, + "checkout_ip": {"varchar", 45, "YES", false}, + "proof_digest": {"char", 64, "YES", false}, + "base_file_id": {"char", 40, "YES", false}, + "lease_expires_at": {"datetime", 0, "YES", false}, + "created_at": {"datetime", 0, "NO", false}, + "updated_at": {"datetime", 0, "NO", false}, +} +var cfV2GuardIndexes = map[string][]cfGuardIndexPart{ + "PRIMARY": {{"repo_id", 0, false}, {"path_hash", 0, false}}, + "cf_edit_guard_id": {{"guard_id", 0, false}}, + "cf_edit_guard_active": {{"repo_id", 1, false}, {"guard_id", 1, false}}, + "cf_edit_guard_owner": {{"owner_uid", 1, false}, {"guard_id", 1, false}}, +} + +// Keep the actual column/index contract in one place and independent of SQL +// diagnostics. No user/lease rows are read and no CREATE/ALTER is issued. +func cfV2GuardValidate(columns map[string]cfGuardColumn, indexes map[string][]cfGuardIndexPart) error { + if len(columns) != len(cfV2GuardColumns) { + return fmt.Errorf("expected %d V2 Guard columns, found %d", len(cfV2GuardColumns), len(columns)) + } + for name, want := range cfV2GuardColumns { + got, exists := columns[name] + if !exists || got != want { + return fmt.Errorf("Guard column %s is missing or incompatible", name) + } + } + for name, want := range cfV2GuardIndexes { + got := indexes[name] + if len(got) != len(want) { + return fmt.Errorf("Guard index %s is missing or incompatible", name) + } + for i := range want { + if got[i] != want[i] { + return fmt.Errorf("Guard index %s has incompatible key order or uniqueness", name) + } + } + } + return nil +} + +func cfV2GuardRequireCurrent(ctx context.Context, db *sql.DB) error { + rows, err := db.QueryContext(ctx, + "SELECT COLUMN_NAME,DATA_TYPE,CHARACTER_MAXIMUM_LENGTH,IS_NULLABLE,COLUMN_TYPE "+ + "FROM information_schema.COLUMNS WHERE TABLE_SCHEMA=DATABASE() AND TABLE_NAME='cf_edit_guard'") + if err != nil { + return fmt.Errorf("cannot inspect Guard columns: %w", err) + } + columns := make(map[string]cfGuardColumn) + for rows.Next() { + var name, kind, nullable, fullKind string + var length sql.NullInt64 + if err := rows.Scan(&name, &kind, &length, &nullable, &fullKind); err != nil { + rows.Close() + return fmt.Errorf("cannot read Guard column definition: %w", err) + } + columns[name] = cfGuardColumn{kind, length.Int64, nullable, + strings.Contains(strings.ToLower(fullKind), "unsigned")} + } + err = rows.Err() + rows.Close() + if err != nil { + return fmt.Errorf("cannot finish Guard column inspection: %w", err) + } + + rows, err = db.QueryContext(ctx, + "SELECT INDEX_NAME,SEQ_IN_INDEX,COLUMN_NAME,NON_UNIQUE,SUB_PART "+ + "FROM information_schema.STATISTICS "+ + "WHERE TABLE_SCHEMA=DATABASE() AND TABLE_NAME='cf_edit_guard' "+ + "ORDER BY INDEX_NAME,SEQ_IN_INDEX") + if err != nil { + return fmt.Errorf("cannot inspect Guard indexes: %w", err) + } + indexes := make(map[string][]cfGuardIndexPart) + for rows.Next() { + var key, name string + var order, nonUnique int64 + var prefix sql.NullInt64 + if err := rows.Scan(&key, &order, &name, &nonUnique, &prefix); err != nil { + rows.Close() + return fmt.Errorf("cannot read Guard index definition: %w", err) + } + if order != int64(len(indexes[key])+1) { + rows.Close() + return fmt.Errorf("Guard index %s key order is invalid", key) + } + indexes[key] = append(indexes[key], cfGuardIndexPart{name, nonUnique, prefix.Valid}) + } + err = rows.Err() + rows.Close() + if err != nil { + return fmt.Errorf("cannot finish Guard index inspection: %w", err) + } + return cfV2GuardValidate(columns, indexes) +} diff --git a/fileserver/cf_v2_edit_schema_test.go b/fileserver/cf_v2_edit_schema_test.go new file mode 100644 index 00000000..86eec450 --- /dev/null +++ b/fileserver/cf_v2_edit_schema_test.go @@ -0,0 +1,71 @@ +package main + +import ( + "errors" + "fmt" + "testing" +) + +func guardLayout() (map[string]cfGuardColumn, map[string][]cfGuardIndexPart) { + columns := make(map[string]cfGuardColumn) + for name, column := range cfV2GuardColumns { + columns[name] = column + } + indexes := make(map[string][]cfGuardIndexPart) + for name, parts := range cfV2GuardIndexes { + indexes[name] = append([]cfGuardIndexPart(nil), parts...) + } + return columns, indexes +} + +func TestCFV2GuardSchemaContract(t *testing.T) { + columns, indexes := guardLayout() + if err := cfV2GuardValidate(columns, indexes); err != nil { + t.Fatalf("current V2 Guard rejected: %v", err) + } + for _, tc := range []struct { + name string + edit func(map[string]cfGuardColumn, map[string][]cfGuardIndexPart) + }{ + {"old lifecycle_ref instead of path", func(c map[string]cfGuardColumn, _ map[string][]cfGuardIndexPart) { + delete(c, "path") + c["lifecycle_ref"] = cfGuardColumn{kind: "varchar", length: 512, nullable: "NO"} + }}, + {"old lease_until instead of lease_expires_at", func(c map[string]cfGuardColumn, _ map[string][]cfGuardIndexPart) { + delete(c, "lease_expires_at") + c["lease_until"] = cfGuardColumn{kind: "datetime", nullable: "YES"} + }}, + {"path hash stored as text", func(c map[string]cfGuardColumn, _ map[string][]cfGuardIndexPart) { + c["path_hash"] = cfGuardColumn{kind: "char", length: 64, nullable: "NO"} + }}, + {"missing active index", func(_ map[string]cfGuardColumn, idx map[string][]cfGuardIndexPart) { + delete(idx, "cf_edit_guard_active") + }}, + {"reversed active index", func(_ map[string]cfGuardColumn, idx map[string][]cfGuardIndexPart) { + idx["cf_edit_guard_active"][0], idx["cf_edit_guard_active"][1] = + idx["cf_edit_guard_active"][1], idx["cf_edit_guard_active"][0] + }}, + {"old resource key", func(_ map[string]cfGuardColumn, idx map[string][]cfGuardIndexPart) { + idx["PRIMARY"] = []cfGuardIndexPart{{name: "resource_uid", nonUnique: 0}} + }}, + } { + t.Run(tc.name, func(t *testing.T) { + c, i := guardLayout() + tc.edit(c, i) + if err := cfV2GuardValidate(c, i); err == nil { + t.Fatal("outdated or incompatible Guard layout passed") + } + }) + } +} + +func TestCFUploadRetriesOnlyHeadConflict(t *testing.T) { + if !cfBranchConflictRetryable(ErrConflict) || !cfBranchConflictRetryable(fmt.Errorf("wrapped: %w", ErrConflict)) { + t.Fatal("genuine concurrent HEAD moves must remain retryable") + } + for _, err := range []error{errCFEditGuard, errCFACL, errors.New("Error 1176 (42000): Key 'cf_edit_guard_active' doesn't exist in table"), errors.New("SQL unavailable")} { + if cfBranchConflictRetryable(err) { + t.Fatalf("deterministic failure must never retry: %v", err) + } + } +} diff --git a/fileserver/fileop.go b/fileserver/fileop.go index 6b0cfae6..e8fcfbf7 100644 --- a/fileserver/fileop.go +++ b/fileserver/fileop.go @@ -2208,6 +2208,12 @@ func fastForwardOrMerge(user, token string, repo *repomgr.Repo, base, newCommit return nil } +// The only commit failure worth retrying is a concurrent HEAD move. +// Schema drift, ACL/Guard denial and SQL/IO failures cannot be fixed by sleep. +func cfBranchConflictRetryable(err error) bool { + return errors.Is(err, ErrConflict) +} + func genCommitNeedRetry(repo *repomgr.Repo, base *commitmgr.Commit, commit *commitmgr.Commit, newRoot, user string, handleConncurrentUpdate bool, commitID *string, lastGCID string, checkGC bool) (bool, error) { var secondParentID string repoID := repo.ID @@ -2265,11 +2271,15 @@ func genCommitNeedRetry(repo *repomgr.Repo, base *commitmgr.Commit, commit *comm if gcConflict { return false, err } - if err == errCFACL { - return false, err - } + // Only an actual Branch HEAD compare-and-swap conflict is retryable. + // SQL/Guard/ACL failures (including missing cf_edit_guard columns/indexes) + // are deterministic until the deployment is repaired. Never spend ten + // upload retries turning a schema fault into "Concurrent upload". if err != nil { - return true, nil + if cfBranchConflictRetryable(err) { + return true, nil + } + return false, err } if commitID != nil { @@ -2341,8 +2351,8 @@ func updateBranch(repoID, originRepoID, newCommitID, oldCommitID, secondParentID } if oldCommitID != commitID { trans.Rollback() - err := fmt.Errorf("head commit id has changed") - return false, err + // The only retryable updateBranch result: another writer moved HEAD. + return false, ErrConflict } // Final and transactional: an upload preflight cannot protect against a diff --git a/fileserver/fileserver.go b/fileserver/fileserver.go index 88496303..c8e7a933 100644 --- a/fileserver/fileserver.go +++ b/fileserver/fileserver.go @@ -173,6 +173,15 @@ func detectV2Guard() { if err != nil || count < 0 || count > 1 { log.Fatalf("Failed to determine installed CloudFile Checkout Guard: %v (count=%d)", err, count) } + if count == 1 { + // The old 029_editing_core table has the same name but different keys, + // path, lease and indexes. Table existence is not a schema contract. + // Fail startup with an actionable error instead of allowing every + // ordinary upload to retry a deterministic SQL 1176 ten times. + if err := cfV2GuardRequireCurrent(ctx, seafileDB); err != nil { + log.Fatalf("CF_EDIT_SCHEMA_MISMATCH: cf_edit_guard is not the current V2 Guard layout; preserve the old table and run the offline schema plan before restarting the fileserver: %v", err) + } + } option.V2GuardAvailable = count == 1 } diff --git a/server/cloudfile-policy.c b/server/cloudfile-policy.c index 5ee96780..360a107f 100644 --- a/server/cloudfile-policy.c +++ b/server/cloudfile-policy.c @@ -218,21 +218,21 @@ lease_resource_location (SeafDBTrans *trans, const char *repo, return count >= 0 && count <= 1 && location.valid && (!uid || count == 1) ? 0 : -1; } -/* Only the opt-in lock barrier requires the new checkout schema. The final - * Branch transaction must re-read it after PREPARE, closing that race for - * scoped file publication. A missing or malformed table fails closed. */ +/* Native C final-publication Guard: current V2 single-table schema only. + * A checked-out file is reserved from all ordinary writers, even the owner; + * Agent-controlled edit publication uses a separate proof-bearing flow. + * The caller holds Branch(master) FOR UPDATE in this same transaction. + * SQL/layout errors deny rather than becoming concurrent commit retries. */ static int editing_conflict (SeafDBTrans *trans, const char *repo, const char *path, - const char *hash, const char *user) + const char *hash) { gboolean error = FALSE; gboolean guarded = seaf_db_trans_check_for_existence (trans, - "SELECT r.uid FROM cf_resource r JOIN cf_edit_guard g ON g.resource_uid=r.uid " - "WHERE r.repo_id=? AND r.path_hash=? AND r.path=? AND r.state='active' " - "AND g.repo_id=r.repo_id AND g.guard_id IS NOT NULL " - "AND (g.lifecycle_ref<>r.lifecycle_ref OR g.mode IS NULL OR g.mode<>'file-lock' OR g.owner_native_user IS NULL OR g.owner_native_user<>?) FOR UPDATE", - &error, 4, "string", repo, "string", hash, "string", path, - "string", user ? user : ""); + "SELECT path FROM cf_edit_guard WHERE repo_id=? AND path_hash=UNHEX(?) " + "AND path=? AND guard_id IS NOT NULL " + "AND lease_expires_at>UTC_TIMESTAMP(6) FOR UPDATE", + &error, 3, "string", repo, "string", hash, "string", path); return guarded || error ? -1 : 0; } @@ -240,22 +240,13 @@ int cf_policy_check_unleased_write (SeafDBTrans *trans, const char *repo, const char *path, const char *user) { + (void)user; if (!cf_lock_enabled ()) return 0; - if (!repo || !path || path[0] != '/' || strlen (path) > 4096 || - !g_utf8_validate (path, -1, NULL) || editing_schema (trans) < 0) return -1; + if (!trans || !repo || !path || path[0] != '/' || + strlen (path) > 4096 || !g_utf8_validate (path, -1, NULL)) return -1; char *digest = g_compute_checksum_for_string (G_CHECKSUM_SHA256, path, -1); if (!digest) return -1; - if (lease_resource_location (trans, repo, path, digest, NULL) < 0) { - g_free (digest); - return -1; - } - /* Current locking read, not a preflight/RR snapshot. The actual sparse UID - * and lease rows stay locked until the caller's Branch transaction ends. - * SQL errors fail closed; orphan/lifecycle reconciliation remains a - * separate native integration prerequisite, not proven by this join. - * Exact stored path is compared after hashing to avoid digest-only identity. - */ - int result = editing_conflict (trans, repo, path, digest, user); + int result = editing_conflict (trans, repo, path, digest); g_free (digest); return result; }