Monorepo for Tangled
appview/middleware: fix renames that collide w/ live repo's slug
Lewis: May this revision serve well! <lewis@tangled.org>
+148
-3
+6
appview/ingester_repo.go
+6
appview/ingester_repo.go
···
101
101
if err := db.RecordRepoRename(tx, e.Did, prev.Rkey, repoDid); err != nil {
102
102
return fmt.Errorf("failed to record rename history: %w", err)
103
103
}
104
+
if err := db.DeleteRepoRename(tx, e.Did, strings.ToLower(newName)); err != nil {
105
+
return fmt.Errorf("failed to clear colliding rename alias: %w", err)
106
+
}
104
107
105
108
renamed := *prev
106
109
renamed.Rkey = e.Commit.RKey
···
159
162
160
163
if err := db.AddRepo(tx, repo); err != nil {
161
164
return fmt.Errorf("failed to insert repo: %w", err)
165
+
}
166
+
if err := db.DeleteRepoRename(tx, e.Did, strings.ToLower(repo.Slug())); err != nil {
167
+
return fmt.Errorf("failed to clear colliding rename alias: %w", err)
162
168
}
163
169
if err := tx.Commit(); err != nil {
164
170
return fmt.Errorf("failed to commit insert tx: %w", err)
+52
appview/ingester_repo_test.go
+52
appview/ingester_repo_test.go
···
807
807
t.Errorf("metadata from repoDid-mutating update applied: %+v", akshay)
808
808
}
809
809
}
810
+
811
+
func renameAliasExists(t *testing.T, ing *Ingester, ownerDid, oldRkey string) bool {
812
+
t.Helper()
813
+
var n int
814
+
if err := ing.Db.QueryRow(
815
+
`select count(*) from repo_renames where owner_did = ? and old_rkey = ?`,
816
+
ownerDid, oldRkey,
817
+
).Scan(&n); err != nil {
818
+
t.Fatalf("count repo_renames %q: %v", oldRkey, err)
819
+
}
820
+
return n > 0
821
+
}
822
+
823
+
func TestIngestRepo_RenameClearsCollidingAlias(t *testing.T) {
824
+
ing, _ := newTestIngester(t)
825
+
seedRepoRow(t, ing, "did:plc:akshay", "knot.example", "anemone-old", "anemone-old", "did:plc:anemone")
826
+
if err := db.RecordRepoRename(ing.Db, "did:plc:akshay", "anemone", "did:plc:anemone"); err != nil {
827
+
t.Fatalf("RecordRepoRename: %v", err)
828
+
}
829
+
830
+
e := makeEvent(t, jmodels.CommitOperationCreate, "did:plc:akshay", "3mpxmsvicr2zn", tangled.Repo{
831
+
Knot: "knot.example", Name: ptr("anemone"), RepoDid: ptr("did:plc:anemone"),
832
+
})
833
+
if err := ingestAcceptingOwner(t, ing, e); err != nil {
834
+
t.Fatalf("ingestRepo: %v", err)
835
+
}
836
+
837
+
if renameAliasExists(t, ing, "did:plc:akshay", "anemone") {
838
+
t.Error("alias equal to the new live slug must be cleared to avoid a self-redirect loop")
839
+
}
840
+
if !renameAliasExists(t, ing, "did:plc:akshay", "anemone-old") {
841
+
t.Error("alias for the prior slug must be recorded and survive")
842
+
}
843
+
}
844
+
845
+
func TestIngestRepo_InsertClearsCollidingAlias(t *testing.T) {
846
+
ing, _ := newTestIngester(t)
847
+
if err := db.RecordRepoRename(ing.Db, "did:plc:akshay", "clam", "did:plc:clams-former-repo"); err != nil {
848
+
t.Fatalf("RecordRepoRename: %v", err)
849
+
}
850
+
851
+
e := makeEvent(t, jmodels.CommitOperationCreate, "did:plc:akshay", "3mpxmfgowwck3", tangled.Repo{
852
+
Knot: "knot.example", Name: ptr("clam"), RepoDid: ptr("did:plc:clam-repo"),
853
+
})
854
+
if err := ingestAcceptingOwner(t, ing, e); err != nil {
855
+
t.Fatalf("ingestRepo: %v", err)
856
+
}
857
+
858
+
if renameAliasExists(t, ing, "did:plc:akshay", "clam") {
859
+
t.Error("stale alias must be cleared when a live repo claims that slug")
860
+
}
861
+
}
+7
-3
appview/middleware/middleware.go
+7
-3
appview/middleware/middleware.go
···
8
8
"log/slog"
9
9
"net/http"
10
10
"net/url"
11
+
"path"
11
12
"slices"
12
13
"strconv"
13
14
"strings"
···
287
288
if id.Handle.IsInvalidHandle() || handle == "" {
288
289
handle = id.DID.String()
289
290
}
290
-
target := reporesolver.CanonicalRedirectTarget(req, reporesolver.CanonicalRepoPath(handle, repo))
291
-
http.Redirect(w, req, target, http.StatusMovedPermanently)
292
-
return
291
+
canonical := reporesolver.CanonicalRepoPath(handle, repo)
292
+
if path.Join(chi.URLParam(req, "user"), repoName) != canonical {
293
+
target := reporesolver.CanonicalRedirectTarget(req, canonical)
294
+
http.Redirect(w, req, target, http.StatusMovedPermanently)
295
+
return
296
+
}
293
297
}
294
298
295
299
ctx := context.WithValue(req.Context(), "repo", repo)
+83
appview/middleware/resolve_repo_test.go
+83
appview/middleware/resolve_repo_test.go
···
1
+
package middleware
2
+
3
+
import (
4
+
"context"
5
+
"io"
6
+
"log/slog"
7
+
"net/http"
8
+
"net/http/httptest"
9
+
"path/filepath"
10
+
"testing"
11
+
12
+
"github.com/bluesky-social/indigo/atproto/identity"
13
+
"github.com/bluesky-social/indigo/atproto/syntax"
14
+
"github.com/go-chi/chi/v5"
15
+
"tangled.org/core/appview/db"
16
+
"tangled.org/core/appview/models"
17
+
)
18
+
19
+
func TestResolveRepo_RenameAlias(t *testing.T) {
20
+
const ownerDid, handle, knot = "did:plc:boltless", "boltless.dev", "knot1.tangled.sh"
21
+
cases := []struct {
22
+
name string
23
+
repoName, repoRkey, did string
24
+
alias, reqRepo string
25
+
wantLocation string // empty => expect the repo to be served without a redirect
26
+
}{
27
+
{"alias equal to live slug is served, not looped", "anemone", "3mpxmsvicr2zn", "did:plc:anemone", "anemone", "anemone", ""},
28
+
{"genuine rename still redirects", "whelk", "whelk", "did:plc:whelk", "conch", "conch", "/boltless.dev/whelk"},
29
+
}
30
+
31
+
for _, tc := range cases {
32
+
t.Run(tc.name, func(t *testing.T) {
33
+
d, err := db.Make(context.Background(), filepath.Join(t.TempDir(), "test.db"))
34
+
if err != nil {
35
+
t.Fatalf("Make: %v", err)
36
+
}
37
+
t.Cleanup(func() { d.Close() })
38
+
39
+
tx, err := d.Begin()
40
+
if err != nil {
41
+
t.Fatalf("Begin: %v", err)
42
+
}
43
+
if err := db.AddRepo(tx, &models.Repo{Did: ownerDid, Name: tc.repoName, Knot: knot, Rkey: tc.repoRkey, RepoDid: tc.did}); err != nil {
44
+
t.Fatalf("AddRepo: %v", err)
45
+
}
46
+
if err := tx.Commit(); err != nil {
47
+
t.Fatalf("Commit: %v", err)
48
+
}
49
+
if err := db.RecordRepoRename(d, ownerDid, tc.alias, tc.did); err != nil {
50
+
t.Fatalf("RecordRepoRename: %v", err)
51
+
}
52
+
53
+
req := httptest.NewRequest(http.MethodGet, "/"+handle+"/"+tc.reqRepo, nil)
54
+
rctx := chi.NewRouteContext()
55
+
rctx.URLParams.Add("user", handle)
56
+
rctx.URLParams.Add("repo", tc.reqRepo)
57
+
ctx := context.WithValue(req.Context(), chi.RouteCtxKey, rctx)
58
+
ctx = context.WithValue(ctx, "resolvedId", identity.Identity{DID: syntax.DID(ownerDid), Handle: syntax.Handle(handle)})
59
+
60
+
rec := httptest.NewRecorder()
61
+
served := false
62
+
next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { served = true })
63
+
mw := Middleware{db: d, logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
64
+
mw.ResolveRepo()(next).ServeHTTP(rec, req.WithContext(ctx))
65
+
66
+
if tc.wantLocation == "" {
67
+
if !served {
68
+
t.Fatalf("expected repo served, got code=%d Location=%q", rec.Code, rec.Header().Get("Location"))
69
+
}
70
+
return
71
+
}
72
+
if served {
73
+
t.Fatal("expected a redirect, but the repo was served")
74
+
}
75
+
if rec.Code != http.StatusMovedPermanently {
76
+
t.Fatalf("got %d, want 301", rec.Code)
77
+
}
78
+
if got := rec.Header().Get("Location"); got != tc.wantLocation {
79
+
t.Errorf("Location = %q, want %q", got, tc.wantLocation)
80
+
}
81
+
})
82
+
}
83
+
}