Skip to content

Commit 3563ff7

Browse files
Address Tangled review feedback
1 parent f654e7e commit 3563ff7

4 files changed

Lines changed: 73 additions & 2 deletions

File tree

tangled/branches.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ func (s *branchService) List(ctx context.Context, owner, repo string, opts forge
1717
if err != nil {
1818
return nil, err
1919
}
20+
return s.list(ctx, repoDID, opts)
21+
}
2022

23+
func (s *branchService) list(ctx context.Context, repoDID string, opts forges.ListBranchOpts) ([]forges.Branch, error) {
2124
var branches []forges.Branch
2225
cursor := ""
2326
for {

tangled/repos.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,14 @@ func (s *repoService) Get(ctx context.Context, owner, repo string) (*forges.Repo
3131
HasIssues: true,
3232
PullRequestsEnabled: true,
3333
}
34-
if branches, err := s.f.Branches().List(ctx, owner, repo, forges.ListBranchOpts{Limit: 1}); err == nil && len(branches) > 0 {
34+
repoDID := meta.DID
35+
if repoDID == "" && strings.HasPrefix(owner, "did:") {
36+
repoDID = owner
37+
}
38+
if repoDID == "" {
39+
return result, nil
40+
}
41+
if branches, err := (&branchService{f: s.f}).list(ctx, repoDID, forges.ListBranchOpts{Limit: 1}); err == nil && len(branches) > 0 {
3542
result.DefaultBranch = branches[0].Name
3643
}
3744
return result, nil

tangled/tangled.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const (
2121
xrpcListBranches = "sh.tangled.git.temp.listBranches"
2222
xrpcListTags = "sh.tangled.git.temp.listTags"
2323
xrpcGetTree = "sh.tangled.git.temp.getTree"
24+
maxRepoHTMLBytes = 1 << 20
2425
)
2526

2627
type tangledForge struct {
@@ -154,13 +155,24 @@ func (f *tangledForge) repoMeta(ctx context.Context, owner, repo string) (*repoM
154155
return nil, &forges.HTTPError{StatusCode: resp.StatusCode, URL: req.URL.String(), Body: string(body)}
155156
}
156157

157-
body, err := io.ReadAll(resp.Body)
158+
body, err := readLimited(resp.Body, maxRepoHTMLBytes)
158159
if err != nil {
159160
return nil, err
160161
}
161162
return parseRepoMeta(string(body)), nil
162163
}
163164

165+
func readLimited(r io.Reader, limit int64) ([]byte, error) {
166+
body, err := io.ReadAll(io.LimitReader(r, limit+1))
167+
if err != nil {
168+
return nil, err
169+
}
170+
if int64(len(body)) > limit {
171+
return nil, fmt.Errorf("tangled repository metadata response exceeds %d bytes", limit)
172+
}
173+
return body, nil
174+
}
175+
164176
func (f *tangledForge) repoDID(ctx context.Context, owner, repo string) (string, error) {
165177
if strings.HasPrefix(owner, "did:") {
166178
return owner, nil

tangled/tangled_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"net/http"
77
"net/http/httptest"
8+
"strings"
89
"testing"
910

1011
forges "github.com/git-pkgs/forge"
@@ -32,6 +33,54 @@ func TestRepoGetUsesAppviewMetadataAndBranches(t *testing.T) {
3233
}
3334
}
3435

36+
func TestRepoGetReusesMetadataDIDForDefaultBranch(t *testing.T) {
37+
var metadataRequests int
38+
mux := http.NewServeMux()
39+
mux.HandleFunc("GET /tangled.org/core", func(w http.ResponseWriter, r *http.Request) {
40+
metadataRequests++
41+
_, _ = fmt.Fprintf(w, `<meta name="vcs:clone" content="%s/tangled.org/core">
42+
<body data-star-subject-at="at://did:plc:owner/sh.tangled.repo/core"></body>`, "http://"+r.Host)
43+
})
44+
mux.HandleFunc("GET /xrpc/sh.tangled.git.temp.listBranches", func(w http.ResponseWriter, r *http.Request) {
45+
if got := r.URL.Query().Get("repo"); got != "did:plc:owner" {
46+
t.Errorf("repo query = %q", got)
47+
}
48+
_, _ = fmt.Fprint(w, `{"branches":[{"name":"master"}]}`)
49+
})
50+
srv := httptest.NewServer(mux)
51+
defer srv.Close()
52+
53+
f := New(srv.URL, "", srv.Client())
54+
repo, err := f.Repos().Get(context.Background(), "tangled.org", "core")
55+
if err != nil {
56+
t.Fatalf("Get returned error: %v", err)
57+
}
58+
if repo.DefaultBranch != "master" {
59+
t.Errorf("DefaultBranch = %q", repo.DefaultBranch)
60+
}
61+
if metadataRequests != 1 {
62+
t.Errorf("metadata requests = %d, want 1", metadataRequests)
63+
}
64+
}
65+
66+
func TestRepoGetRejectsOversizedMetadata(t *testing.T) {
67+
mux := http.NewServeMux()
68+
mux.HandleFunc("GET /tangled.org/core", func(w http.ResponseWriter, r *http.Request) {
69+
_, _ = w.Write([]byte(strings.Repeat("x", maxRepoHTMLBytes+1)))
70+
})
71+
srv := httptest.NewServer(mux)
72+
defer srv.Close()
73+
74+
f := New(srv.URL, "", srv.Client())
75+
_, err := f.Repos().Get(context.Background(), "tangled.org", "core")
76+
if err == nil {
77+
t.Fatal("expected oversized metadata error")
78+
}
79+
if !strings.Contains(err.Error(), "exceeds") {
80+
t.Errorf("expected size limit error, got %v", err)
81+
}
82+
}
83+
3584
func TestBranchListUsesTangledXRPC(t *testing.T) {
3685
srv := tangledTestServer(t)
3786
f := New(srv.URL, "", srv.Client())

0 commit comments

Comments
 (0)