From 8dd9e59b99e83e757bcadf11a4bda31fc78e6732 Mon Sep 17 00:00:00 2001 From: oppiliappan Date: Tue, 09 Sep 2025 08:14:35 +0000 Subject: [PATCH] appview,knotserver: make ref optional in all xrpc endpoint this is backwards compatible mostly. there are bugs in the old handlers around refs that include url escaped characters, these have been remedied with this patch. Signed-off-by: oppiliappan --- appview/repo/index.go | 16 +++++++++------- appview/repo/repo.go | 215 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------------------------------------------------- knotserver/xrpc/repo_archive.go | 10 +++++++--- knotserver/xrpc/repo_blob.go | 6 +++++- knotserver/xrpc/repo_branch.go | 5 +++-- knotserver/xrpc/repo_branches.go | 5 +---- knotserver/xrpc/repo_compare.go | 12 ++++-------- knotserver/xrpc/repo_diff.go | 13 ++----------- knotserver/xrpc/repo_get_default_branch.go | 12 +++--------- knotserver/xrpc/repo_languages.go | 9 ++------- knotserver/xrpc/repo_log.go | 19 +------------------ knotserver/xrpc/repo_tags.go | 4 ++-- knotserver/xrpc/repo_tree.go | 23 ++++------------------- knotserver/xrpc/xrpc.go | 31 ++++--------------------------- appview/pages/templates/repo/tree.html | 6 +++--- 15 file(s) changed, 147 insertion(s)(+), 239 deletion(s)(-) diff --git a/appview/repo/index.go b/appview/repo/index.go --- a/appview/repo/index.go +++ b/appview/repo/index.go @@ -5,6 +5,7 @@ "fmt" "log" "net/http" + "net/url" "slices" "sort" "strings" @@ -31,6 +32,7 @@ func (rp *Repo) RepoIndex(w http.ResponseWriter, r *http.Request) { ref := chi.URLParam(r, "ref") + ref, _ = url.PathUnescape(ref) f, err := rp.repoResolver.Resolve(r) if err != nil { @@ -245,12 +247,12 @@ // first get branches to determine the ref if not specified branchesBytes, err := tangled.RepoBranches(ctx, xrpcc, "", 0, repo) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to call repoBranches: %w", err) } var branchesResp types.RepoBranchesResponse if err := json.Unmarshal(branchesBytes, &branchesResp); err != nil { - return nil, err + return nil, fmt.Errorf("failed to unmarshal branches response: %w", err) } // if no ref specified, use default branch or first available @@ -292,12 +294,12 @@ defer wg.Done() tagsBytes, err := tangled.RepoTags(ctx, xrpcc, "", 0, repo) if err != nil { - errs = errors.Join(errs, err) + errs = errors.Join(errs, fmt.Errorf("failed to call repoTags: %w", err)) return } if err := json.Unmarshal(tagsBytes, &tagsResp); err != nil { - errs = errors.Join(errs, err) + errs = errors.Join(errs, fmt.Errorf("failed to unmarshal repoTags: %w", err)) } }() @@ -307,7 +309,7 @@ defer wg.Done() resp, err := tangled.RepoTree(ctx, xrpcc, "", ref, repo) if err != nil { - errs = errors.Join(errs, err) + errs = errors.Join(errs, fmt.Errorf("failed to call repoTree: %w", err)) return } treeResp = resp @@ -319,12 +321,12 @@ defer wg.Done() logBytes, err := tangled.RepoLog(ctx, xrpcc, "", 50, "", ref, repo) if err != nil { - errs = errors.Join(errs, err) + errs = errors.Join(errs, fmt.Errorf("failed to call repoLog: %w", err)) return } if err := json.Unmarshal(logBytes, &logResp); err != nil { - errs = errors.Join(errs, err) + errs = errors.Join(errs, fmt.Errorf("failed to unmarshal repoLog: %w", err)) } }() diff --git a/appview/repo/repo.go b/appview/repo/repo.go --- a/appview/repo/repo.go +++ b/appview/repo/repo.go @@ -85,7 +85,9 @@ } func (rp *Repo) DownloadArchive(w http.ResponseWriter, r *http.Request) { - refParam := chi.URLParam(r, "ref") + ref := chi.URLParam(r, "ref") + ref, _ = url.PathUnescape(ref) + f, err := rp.repoResolver.Resolve(r) if err != nil { log.Println("failed to get repo and knot", err) @@ -102,19 +104,16 @@ } repo := fmt.Sprintf("%s/%s", f.OwnerDid(), f.Name) - archiveBytes, err := tangled.RepoArchive(r.Context(), xrpcc, "tar.gz", "", refParam, repo) - if err != nil { - if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { - log.Println("failed to call XRPC repo.archive", xrpcerr) - rp.pages.Error503(w) - return - } - rp.pages.Error404(w) + archiveBytes, err := tangled.RepoArchive(r.Context(), xrpcc, "tar.gz", "", ref, repo) + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + log.Println("failed to call XRPC repo.archive", xrpcerr) + rp.pages.Error503(w) return } - // Set headers for file download - filename := fmt.Sprintf("%s-%s.tar.gz", f.Name, refParam) + // Set headers for file download, just pass along whatever the knot specifies + safeRefFilename := strings.ReplaceAll(plumbing.ReferenceName(ref).Short(), "/", "-") + filename := fmt.Sprintf("%s-%s.tar.gz", f.Name, safeRefFilename) w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=\"%s\"", filename)) w.Header().Set("Content-Type", "application/gzip") w.Header().Set("Content-Length", fmt.Sprintf("%d", len(archiveBytes))) @@ -139,6 +138,7 @@ } ref := chi.URLParam(r, "ref") + ref, _ = url.PathUnescape(ref) scheme := "http" if !rp.config.Core.Dev { @@ -159,13 +159,9 @@ repo := fmt.Sprintf("%s/%s", f.OwnerDid(), f.Name) xrpcBytes, err := tangled.RepoLog(r.Context(), xrpcc, cursor, limit, "", ref, repo) - if err != nil { - if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { - log.Println("failed to call XRPC repo.log", xrpcerr) - rp.pages.Error503(w) - return - } - rp.pages.Error404(w) + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + log.Println("failed to call XRPC repo.log", xrpcerr) + rp.pages.Error503(w) return } @@ -177,12 +173,10 @@ } tagBytes, err := tangled.RepoTags(r.Context(), xrpcc, "", 0, repo) - if err != nil { - if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { - log.Println("failed to call XRPC repo.tags", xrpcerr) - rp.pages.Error503(w) - return - } + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + log.Println("failed to call XRPC repo.tags", xrpcerr) + rp.pages.Error503(w) + return } tagMap := make(map[string][]string) @@ -196,12 +190,10 @@ } branchBytes, err := tangled.RepoBranches(r.Context(), xrpcc, "", 0, repo) - if err != nil { - if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { - log.Println("failed to call XRPC repo.branches", xrpcerr) - rp.pages.Error503(w) - return - } + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + log.Println("failed to call XRPC repo.branches", xrpcerr) + rp.pages.Error503(w) + return } if branchBytes != nil { @@ -353,6 +345,7 @@ return } ref := chi.URLParam(r, "ref") + ref, _ = url.PathUnescape(ref) var diffOpts types.DiffOpts if d := r.URL.Query().Get("diff"); d == "split" { @@ -375,13 +368,9 @@ repo := fmt.Sprintf("%s/%s", f.OwnerDid(), f.Name) xrpcBytes, err := tangled.RepoDiff(r.Context(), xrpcc, ref, repo) - if err != nil { - if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { - log.Println("failed to call XRPC repo.diff", xrpcerr) - rp.pages.Error503(w) - return - } - rp.pages.Error404(w) + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + log.Println("failed to call XRPC repo.diff", xrpcerr) + rp.pages.Error503(w) return } @@ -433,10 +422,12 @@ } ref := chi.URLParam(r, "ref") - treePath := chi.URLParam(r, "*") + ref, _ = url.PathUnescape(ref) // if the tree path has a trailing slash, let's strip it // so we don't 404 + treePath := chi.URLParam(r, "*") + treePath, _ = url.PathUnescape(treePath) treePath = strings.TrimSuffix(treePath, "/") scheme := "http" @@ -450,13 +441,9 @@ repo := fmt.Sprintf("%s/%s", f.OwnerDid(), f.Name) xrpcResp, err := tangled.RepoTree(r.Context(), xrpcc, treePath, ref, repo) - if err != nil { - if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { - log.Println("failed to call XRPC repo.tree", xrpcerr) - rp.pages.Error503(w) - return - } - rp.pages.Error404(w) + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + log.Println("failed to call XRPC repo.tree", xrpcerr) + rp.pages.Error503(w) return } @@ -498,19 +485,19 @@ // redirects tree paths trying to access a blob; in this case the result.Files is unpopulated, // so we can safely redirect to the "parent" (which is the same file). - unescapedTreePath, _ := url.PathUnescape(treePath) - if len(result.Files) == 0 && result.Parent == unescapedTreePath { - http.Redirect(w, r, fmt.Sprintf("/%s/blob/%s/%s", f.OwnerSlashRepo(), ref, result.Parent), http.StatusFound) + if len(result.Files) == 0 && result.Parent == treePath { + redirectTo := fmt.Sprintf("/%s/blob/%s/%s", f.OwnerSlashRepo(), url.PathEscape(ref), result.Parent) + http.Redirect(w, r, redirectTo, http.StatusFound) return } user := rp.oauth.GetUser(r) var breadcrumbs [][]string - breadcrumbs = append(breadcrumbs, []string{f.Name, fmt.Sprintf("/%s/tree/%s", f.OwnerSlashRepo(), ref)}) + breadcrumbs = append(breadcrumbs, []string{f.Name, fmt.Sprintf("/%s/tree/%s", f.OwnerSlashRepo(), url.PathEscape(ref))}) if treePath != "" { for idx, elem := range strings.Split(treePath, "/") { - breadcrumbs = append(breadcrumbs, []string{elem, fmt.Sprintf("%s/%s", breadcrumbs[idx][1], elem)}) + breadcrumbs = append(breadcrumbs, []string{elem, fmt.Sprintf("%s/%s", breadcrumbs[idx][1], url.PathEscape(elem))}) } } @@ -543,13 +530,9 @@ repo := fmt.Sprintf("%s/%s", f.OwnerDid(), f.Name) xrpcBytes, err := tangled.RepoTags(r.Context(), xrpcc, "", 0, repo) - if err != nil { - if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { - log.Println("failed to call XRPC repo.tags", xrpcerr) - rp.pages.Error503(w) - return - } - rp.pages.Error404(w) + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + log.Println("failed to call XRPC repo.tags", xrpcerr) + rp.pages.Error503(w) return } @@ -616,13 +599,9 @@ repo := fmt.Sprintf("%s/%s", f.OwnerDid(), f.Name) xrpcBytes, err := tangled.RepoBranches(r.Context(), xrpcc, "", 0, repo) - if err != nil { - if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { - log.Println("failed to call XRPC repo.branches", xrpcerr) - rp.pages.Error503(w) - return - } - rp.pages.Error404(w) + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + log.Println("failed to call XRPC repo.branches", xrpcerr) + rp.pages.Error503(w) return } @@ -651,7 +630,10 @@ } ref := chi.URLParam(r, "ref") + ref, _ = url.PathUnescape(ref) + filePath := chi.URLParam(r, "*") + filePath, _ = url.PathUnescape(filePath) scheme := "http" if !rp.config.Core.Dev { @@ -664,23 +646,19 @@ repo := fmt.Sprintf("%s/%s", f.OwnerDid(), f.Repo.Name) resp, err := tangled.RepoBlob(r.Context(), xrpcc, filePath, false, ref, repo) - if err != nil { - if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { - log.Println("failed to call XRPC repo.blob", xrpcerr) - rp.pages.Error503(w) - return - } - rp.pages.Error404(w) + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + log.Println("failed to call XRPC repo.blob", xrpcerr) + rp.pages.Error503(w) return } // Use XRPC response directly instead of converting to internal types var breadcrumbs [][]string - breadcrumbs = append(breadcrumbs, []string{f.Name, fmt.Sprintf("/%s/tree/%s", f.OwnerSlashRepo(), ref)}) + breadcrumbs = append(breadcrumbs, []string{f.Name, fmt.Sprintf("/%s/tree/%s", f.OwnerSlashRepo(), url.PathEscape(ref))}) if filePath != "" { for idx, elem := range strings.Split(filePath, "/") { - breadcrumbs = append(breadcrumbs, []string{elem, fmt.Sprintf("%s/%s", breadcrumbs[idx][1], elem)}) + breadcrumbs = append(breadcrumbs, []string{elem, fmt.Sprintf("%s/%s", breadcrumbs[idx][1], url.PathEscape(elem))}) } } @@ -710,8 +688,19 @@ // fetch the raw binary content using sh.tangled.repo.blob xrpc repoName := fmt.Sprintf("%s/%s", f.OwnerDid(), f.Name) - blobURL := fmt.Sprintf("%s://%s/xrpc/sh.tangled.repo.blob?repo=%s&ref=%s&path=%s&raw=true", - scheme, f.Knot, url.QueryEscape(repoName), url.QueryEscape(ref), url.QueryEscape(filePath)) + + baseURL := &url.URL{ + Scheme: scheme, + Host: f.Knot, + Path: "/xrpc/sh.tangled.repo.blob", + } + query := baseURL.Query() + query.Set("repo", repoName) + query.Set("ref", ref) + query.Set("path", filePath) + query.Set("raw", "true") + baseURL.RawQuery = query.Encode() + blobURL := baseURL.String() contentSrc = blobURL if !rp.config.Core.Dev { @@ -766,7 +755,10 @@ } ref := chi.URLParam(r, "ref") + ref, _ = url.PathUnescape(ref) + filePath := chi.URLParam(r, "*") + filePath, _ = url.PathUnescape(filePath) scheme := "http" if !rp.config.Core.Dev { @@ -774,8 +766,18 @@ } repo := fmt.Sprintf("%s/%s", f.OwnerDid(), f.Repo.Name) - blobURL := fmt.Sprintf("%s://%s/xrpc/sh.tangled.repo.blob?repo=%s&ref=%s&path=%s&raw=true", - scheme, f.Knot, url.QueryEscape(repo), url.QueryEscape(ref), url.QueryEscape(filePath)) + baseURL := &url.URL{ + Scheme: scheme, + Host: f.Knot, + Path: "/xrpc/sh.tangled.repo.blob", + } + query := baseURL.Query() + query.Set("repo", repo) + query.Set("ref", ref) + query.Set("path", filePath) + query.Set("raw", "true") + baseURL.RawQuery = query.Encode() + blobURL := baseURL.String() req, err := http.NewRequest("GET", blobURL, nil) if err != nil { @@ -1364,12 +1366,8 @@ repo := fmt.Sprintf("%s/%s", f.OwnerDid(), f.Name) xrpcBytes, err := tangled.RepoBranches(r.Context(), xrpcc, "", 0, repo) - if err != nil { - if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { - log.Println("failed to call XRPC repo.branches", xrpcerr) - rp.pages.Error503(w) - return - } + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + log.Println("failed to call XRPC repo.branches", xrpcerr) rp.pages.Error503(w) return } @@ -1471,6 +1469,7 @@ func (rp *Repo) SyncRepoFork(w http.ResponseWriter, r *http.Request) { ref := chi.URLParam(r, "ref") + ref, _ = url.PathUnescape(ref) user := rp.oauth.GetUser(r) f, err := rp.repoResolver.Resolve(r) @@ -1759,13 +1758,9 @@ repo := fmt.Sprintf("%s/%s", f.OwnerDid(), f.Name) branchBytes, err := tangled.RepoBranches(r.Context(), xrpcc, "", 0, repo) - if err != nil { - if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { - log.Println("failed to call XRPC repo.branches", xrpcerr) - rp.pages.Error503(w) - return - } - rp.pages.Notice(w, "compare-error", "Failed to produce comparison. Try again later.") + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + log.Println("failed to call XRPC repo.branches", xrpcerr) + rp.pages.Error503(w) return } @@ -1800,13 +1795,9 @@ } tagBytes, err := tangled.RepoTags(r.Context(), xrpcc, "", 0, repo) - if err != nil { - if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { - log.Println("failed to call XRPC repo.tags", xrpcerr) - rp.pages.Error503(w) - return - } - rp.pages.Notice(w, "compare-error", "Failed to produce comparison. Try again later.") + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + log.Println("failed to call XRPC repo.tags", xrpcerr) + rp.pages.Error503(w) return } @@ -1877,13 +1868,9 @@ repo := fmt.Sprintf("%s/%s", f.OwnerDid(), f.Name) branchBytes, err := tangled.RepoBranches(r.Context(), xrpcc, "", 0, repo) - if err != nil { - if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { - log.Println("failed to call XRPC repo.branches", xrpcerr) - rp.pages.Error503(w) - return - } - rp.pages.Notice(w, "compare-error", "Failed to produce comparison. Try again later.") + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + log.Println("failed to call XRPC repo.branches", xrpcerr) + rp.pages.Error503(w) return } @@ -1895,13 +1882,9 @@ } tagBytes, err := tangled.RepoTags(r.Context(), xrpcc, "", 0, repo) - if err != nil { - if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { - log.Println("failed to call XRPC repo.tags", xrpcerr) - rp.pages.Error503(w) - return - } - rp.pages.Notice(w, "compare-error", "Failed to produce comparison. Try again later.") + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + log.Println("failed to call XRPC repo.tags", xrpcerr) + rp.pages.Error503(w) return } @@ -1913,13 +1896,9 @@ } compareBytes, err := tangled.RepoCompare(r.Context(), xrpcc, repo, base, head) - if err != nil { - if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { - log.Println("failed to call XRPC repo.compare", xrpcerr) - rp.pages.Error503(w) - return - } - rp.pages.Notice(w, "compare-error", "Failed to produce comparison. Try again later.") + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + log.Println("failed to call XRPC repo.compare", xrpcerr) + rp.pages.Error503(w) return } diff --git a/knotserver/xrpc/repo_archive.go b/knotserver/xrpc/repo_archive.go --- a/knotserver/xrpc/repo_archive.go +++ b/knotserver/xrpc/repo_archive.go @@ -13,11 +13,15 @@ ) func (x *Xrpc) RepoArchive(w http.ResponseWriter, r *http.Request) { - repo, repoPath, unescapedRef, err := x.parseStandardParams(r) + repo := r.URL.Query().Get("repo") + repoPath, err := x.parseRepoParam(repo) if err != nil { writeError(w, err.(xrpcerr.XrpcError), http.StatusBadRequest) return } + + ref := r.URL.Query().Get("ref") + // ref can be empty (git.Open handles this) format := r.URL.Query().Get("format") if format == "" { @@ -34,7 +38,7 @@ return } - gr, err := git.Open(repoPath, unescapedRef) + gr, err := git.Open(repoPath, ref) if err != nil { writeError(w, xrpcerr.NewXrpcError( xrpcerr.WithTag("RefNotFound"), @@ -46,7 +50,7 @@ repoParts := strings.Split(repo, "/") repoName := repoParts[len(repoParts)-1] - safeRefFilename := strings.ReplaceAll(plumbing.ReferenceName(unescapedRef).Short(), "/", "-") + safeRefFilename := strings.ReplaceAll(plumbing.ReferenceName(ref).Short(), "/", "-") var archivePrefix string if prefix != "" { diff --git a/knotserver/xrpc/repo_blob.go b/knotserver/xrpc/repo_blob.go --- a/knotserver/xrpc/repo_blob.go +++ b/knotserver/xrpc/repo_blob.go @@ -16,11 +16,15 @@ ) func (x *Xrpc) RepoBlob(w http.ResponseWriter, r *http.Request) { - _, repoPath, ref, err := x.parseStandardParams(r) + repo := r.URL.Query().Get("repo") + repoPath, err := x.parseRepoParam(repo) if err != nil { writeError(w, err.(xrpcerr.XrpcError), http.StatusBadRequest) return } + + ref := r.URL.Query().Get("ref") + // ref can be empty (git.Open handles this) treePath := r.URL.Query().Get("path") if treePath == "" { diff --git a/knotserver/xrpc/repo_branch.go b/knotserver/xrpc/repo_branch.go --- a/knotserver/xrpc/repo_branch.go +++ b/knotserver/xrpc/repo_branch.go @@ -4,6 +4,7 @@ "encoding/json" "net/http" "net/url" + "time" "tangled.sh/tangled.sh/core/api/tangled" "tangled.sh/tangled.sh/core/knotserver/git" @@ -70,7 +71,7 @@ Name: ref.Name().Short(), Hash: ref.Hash().String(), ShortHash: &[]string{ref.Hash().String()[:7]}[0], - When: commit.Author.When.Format("2006-01-02T15:04:05.000Z"), + When: commit.Author.When.Format(time.RFC3339), IsDefault: &isDefault, } @@ -81,7 +82,7 @@ response.Author = &tangled.RepoBranch_Signature{ Name: commit.Author.Name, Email: commit.Author.Email, - When: commit.Author.When.Format("2006-01-02T15:04:05.000Z"), + When: commit.Author.When.Format(time.RFC3339), } w.Header().Set("Content-Type", "application/json") diff --git a/knotserver/xrpc/repo_branches.go b/knotserver/xrpc/repo_branches.go --- a/knotserver/xrpc/repo_branches.go +++ b/knotserver/xrpc/repo_branches.go @@ -47,10 +47,7 @@ } } - end := offset + limit - if end > len(branches) { - end = len(branches) - } + end := min(offset+limit, len(branches)) paginatedBranches := branches[offset:end] diff --git a/knotserver/xrpc/repo_compare.go b/knotserver/xrpc/repo_compare.go --- a/knotserver/xrpc/repo_compare.go +++ b/knotserver/xrpc/repo_compare.go @@ -4,7 +4,6 @@ "encoding/json" "fmt" "net/http" - "net/url" "tangled.sh/tangled.sh/core/knotserver/git" "tangled.sh/tangled.sh/core/types" @@ -19,8 +18,8 @@ return } - rev1Param := r.URL.Query().Get("rev1") - if rev1Param == "" { + rev1 := r.URL.Query().Get("rev1") + if rev1 == "" { writeError(w, xrpcerr.NewXrpcError( xrpcerr.WithTag("InvalidRequest"), xrpcerr.WithMessage("missing rev1 parameter"), @@ -28,17 +27,14 @@ return } - rev2Param := r.URL.Query().Get("rev2") - if rev2Param == "" { + rev2 := r.URL.Query().Get("rev2") + if rev2 == "" { writeError(w, xrpcerr.NewXrpcError( xrpcerr.WithTag("InvalidRequest"), xrpcerr.WithMessage("missing rev2 parameter"), ), http.StatusBadRequest) return } - - rev1, _ := url.PathUnescape(rev1Param) - rev2, _ := url.PathUnescape(rev2Param) gr, err := git.PlainOpen(repoPath) if err != nil { diff --git a/knotserver/xrpc/repo_diff.go b/knotserver/xrpc/repo_diff.go --- a/knotserver/xrpc/repo_diff.go +++ b/knotserver/xrpc/repo_diff.go @@ -3,7 +3,6 @@ import ( "encoding/json" "net/http" - "net/url" "tangled.sh/tangled.sh/core/knotserver/git" "tangled.sh/tangled.sh/core/types" @@ -18,16 +17,8 @@ return } - refParam := r.URL.Query().Get("ref") - if refParam == "" { - writeError(w, xrpcerr.NewXrpcError( - xrpcerr.WithTag("InvalidRequest"), - xrpcerr.WithMessage("missing ref parameter"), - ), http.StatusBadRequest) - return - } - - ref, _ := url.QueryUnescape(refParam) + ref := r.URL.Query().Get("ref") + // ref can be empty (git.Open handles this) gr, err := git.Open(repoPath, ref) if err != nil { diff --git a/knotserver/xrpc/repo_get_default_branch.go b/knotserver/xrpc/repo_get_default_branch.go --- a/knotserver/xrpc/repo_get_default_branch.go +++ b/knotserver/xrpc/repo_get_default_branch.go @@ -3,6 +3,7 @@ import ( "encoding/json" "net/http" + "time" "tangled.sh/tangled.sh/core/api/tangled" "tangled.sh/tangled.sh/core/knotserver/git" @@ -17,14 +18,7 @@ return } - gr, err := git.Open(repoPath, "") - if err != nil { - writeError(w, xrpcerr.NewXrpcError( - xrpcerr.WithTag("RepoNotFound"), - xrpcerr.WithMessage("repository not found"), - ), http.StatusNotFound) - return - } + gr, err := git.PlainOpen(repoPath) branch, err := gr.FindMainBranch() if err != nil { @@ -39,7 +33,7 @@ response := tangled.RepoGetDefaultBranch_Output{ Name: branch, Hash: "", - When: "1970-01-01T00:00:00.000Z", + When: time.UnixMicro(0).Format(time.RFC3339), } w.Header().Set("Content-Type", "application/json") diff --git a/knotserver/xrpc/repo_languages.go b/knotserver/xrpc/repo_languages.go --- a/knotserver/xrpc/repo_languages.go +++ b/knotserver/xrpc/repo_languages.go @@ -5,7 +5,6 @@ "encoding/json" "math" "net/http" - "net/url" "time" "tangled.sh/tangled.sh/core/api/tangled" @@ -14,18 +13,14 @@ ) func (x *Xrpc) RepoLanguages(w http.ResponseWriter, r *http.Request) { - refParam := r.URL.Query().Get("ref") - if refParam == "" { - refParam = "HEAD" // default - } - ref, _ := url.PathUnescape(refParam) - repo := r.URL.Query().Get("repo") repoPath, err := x.parseRepoParam(repo) if err != nil { writeError(w, err.(xrpcerr.XrpcError), http.StatusBadRequest) return } + + ref := r.URL.Query().Get("ref") gr, err := git.Open(repoPath, ref) if err != nil { diff --git a/knotserver/xrpc/repo_log.go b/knotserver/xrpc/repo_log.go --- a/knotserver/xrpc/repo_log.go +++ b/knotserver/xrpc/repo_log.go @@ -3,7 +3,6 @@ import ( "encoding/json" "net/http" - "net/url" "strconv" "tangled.sh/tangled.sh/core/knotserver/git" @@ -19,14 +18,7 @@ return } - refParam := r.URL.Query().Get("ref") - if refParam == "" { - writeError(w, xrpcerr.NewXrpcError( - xrpcerr.WithTag("InvalidRequest"), - xrpcerr.WithMessage("missing ref parameter"), - ), http.StatusBadRequest) - return - } + ref := r.URL.Query().Get("ref") path := r.URL.Query().Get("path") cursor := r.URL.Query().Get("cursor") @@ -36,15 +28,6 @@ if l, err := strconv.Atoi(limitStr); err == nil && l > 0 && l <= 100 { limit = l } - } - - ref, err := url.QueryUnescape(refParam) - if err != nil { - writeError(w, xrpcerr.NewXrpcError( - xrpcerr.WithTag("InvalidRequest"), - xrpcerr.WithMessage("invalid ref parameter"), - ), http.StatusBadRequest) - return } gr, err := git.Open(repoPath, ref) diff --git a/knotserver/xrpc/repo_tags.go b/knotserver/xrpc/repo_tags.go --- a/knotserver/xrpc/repo_tags.go +++ b/knotserver/xrpc/repo_tags.go @@ -30,13 +30,13 @@ } } - gr, err := git.Open(repoPath, "") + gr, err := git.PlainOpen(repoPath) if err != nil { x.Logger.Error("failed to open", "error", err) writeError(w, xrpcerr.NewXrpcError( xrpcerr.WithTag("RepoNotFound"), xrpcerr.WithMessage("repository not found"), - ), http.StatusNotFound) + ), http.StatusNoContent) return } diff --git a/knotserver/xrpc/repo_tree.go b/knotserver/xrpc/repo_tree.go --- a/knotserver/xrpc/repo_tree.go +++ b/knotserver/xrpc/repo_tree.go @@ -3,8 +3,8 @@ import ( "encoding/json" "net/http" - "net/url" "path/filepath" + "time" "tangled.sh/tangled.sh/core/api/tangled" "tangled.sh/tangled.sh/core/knotserver/git" @@ -21,26 +21,11 @@ return } - refParam := r.URL.Query().Get("ref") - if refParam == "" { - writeError(w, xrpcerr.NewXrpcError( - xrpcerr.WithTag("InvalidRequest"), - xrpcerr.WithMessage("missing ref parameter"), - ), http.StatusBadRequest) - return - } + ref := r.URL.Query().Get("ref") + // ref can be empty (git.Open handles this) path := r.URL.Query().Get("path") // path can be empty (defaults to root) - - ref, err := url.QueryUnescape(refParam) - if err != nil { - writeError(w, xrpcerr.NewXrpcError( - xrpcerr.WithTag("InvalidRequest"), - xrpcerr.WithMessage("invalid ref parameter"), - ), http.StatusBadRequest) - return - } gr, err := git.Open(repoPath, ref) if err != nil { @@ -77,7 +62,7 @@ entry.Last_commit = &tangled.RepoTree_LastCommit{ Hash: file.LastCommit.Hash.String(), Message: file.LastCommit.Message, - When: file.LastCommit.When.Format("2006-01-02T15:04:05.000Z"), + When: file.LastCommit.When.Format(time.RFC3339), } } diff --git a/knotserver/xrpc/xrpc.go b/knotserver/xrpc/xrpc.go --- a/knotserver/xrpc/xrpc.go +++ b/knotserver/xrpc/xrpc.go @@ -4,7 +4,6 @@ "encoding/json" "log/slog" "net/http" - "net/url" "strings" securejoin "github.com/cyphar/filepath-securejoin" @@ -88,16 +87,16 @@ } // Parse repo string (did/repoName format) - parts := strings.Split(repo, "/") - if len(parts) < 2 { + parts := strings.SplitN(repo, "/", 2) + if len(parts) != 2 { return "", xrpcerr.NewXrpcError( xrpcerr.WithTag("InvalidRequest"), xrpcerr.WithMessage("invalid repo format, expected 'did/repoName'"), ) } - did := strings.Join(parts[:len(parts)-1], "/") - repoName := parts[len(parts)-1] + did := parts[0] + repoName := parts[1] // Construct repository path using the same logic as didPath didRepoPath, err := securejoin.SecureJoin(did, repoName) @@ -117,28 +116,6 @@ } return repoPath, nil -} - -// parseStandardParams parses common query parameters used by most handlers -func (x *Xrpc) parseStandardParams(r *http.Request) (repo, repoPath, ref string, err error) { - // Parse repo parameter - repo = r.URL.Query().Get("repo") - repoPath, err = x.parseRepoParam(repo) - if err != nil { - return "", "", "", err - } - - // Parse and unescape ref parameter - refParam := r.URL.Query().Get("ref") - if refParam == "" { - return "", "", "", xrpcerr.NewXrpcError( - xrpcerr.WithTag("InvalidRequest"), - xrpcerr.WithMessage("missing ref parameter"), - ) - } - - ref, _ = url.QueryUnescape(refParam) - return repo, repoPath, ref, nil } func writeError(w http.ResponseWriter, e xrpcerr.XrpcError, status int) { diff --git a/appview/pages/templates/repo/tree.html b/appview/pages/templates/repo/tree.html --- a/appview/pages/templates/repo/tree.html +++ b/appview/pages/templates/repo/tree.html @@ -25,13 +25,13 @@
{{ range .BreadCrumbs }} - {{ pathUnescape (index . 0) }} / + {{ pathUnescape (index . 0) }} / {{ end }}
{{ $stats := .TreeStats }} - at {{ $.Ref }} + at {{ $.Ref }} {{ if eq $stats.NumFolders 1 }} {{ $stats.NumFolders }} folder @@ -55,7 +55,7 @@ {{ range .Files }}
- {{ $link := printf "/%s/%s/%s/%s/%s" $.RepoInfo.FullName "tree" (urlquery $.Ref) $.TreePath .Name }} + {{ $link := printf "/%s/%s/%s/%s/%s" $.RepoInfo.FullName "tree" (pathEscape $.Ref) $.TreePath .Name }} {{ $icon := "folder" }} {{ $iconStyle := "size-4 fill-current" }} -- tangled.sh