From a64a4880ecf529dc0e5bdef0ad4db572749f5e08 Mon Sep 17 00:00:00 2001 From: oppiliappan Date: Tue, 9 Sep 2025 09:14:35 +0100 Subject: [PATCH] appview,knotserver: make ref optional in all xrpc endpoint Change-Id: xmlpkoozxluxuwtntxykrvyzrpzszksp 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/pages/templates/repo/tree.html | 6 +- 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 +-- 15 files changed, 147 insertions(+), 239 deletions(-) diff --git a/appview/pages/templates/repo/tree.html b/appview/pages/templates/repo/tree.html index 75bd7e7c..a6ff1d15 100644 --- 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" }} diff --git a/appview/repo/index.go b/appview/repo/index.go index 40a9ff5b..c8b9557e 100644 --- a/appview/repo/index.go +++ b/appview/repo/index.go @@ -5,6 +5,7 @@ import ( "fmt" "log" "net/http" + "net/url" "slices" "sort" "strings" @@ -31,6 +32,7 @@ import ( 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 @@ func (rp *Repo) buildIndexResponse(ctx context.Context, xrpcc *indigoxrpc.Client // 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 @@ func (rp *Repo) buildIndexResponse(ctx context.Context, xrpcc *indigoxrpc.Client 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 @@ func (rp *Repo) buildIndexResponse(ctx context.Context, xrpcc *indigoxrpc.Client 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 @@ func (rp *Repo) buildIndexResponse(ctx context.Context, xrpcc *indigoxrpc.Client 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 index bf5dd1dc..ba8e98c2 100644 --- a/appview/repo/repo.go +++ b/appview/repo/repo.go @@ -85,7 +85,9 @@ func New( } 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 @@ func (rp *Repo) DownloadArchive(w http.ResponseWriter, r *http.Request) { } 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 @@ func (rp *Repo) RepoLog(w http.ResponseWriter, r *http.Request) { } ref := chi.URLParam(r, "ref") + ref, _ = url.PathUnescape(ref) scheme := "http" if !rp.config.Core.Dev { @@ -159,13 +159,9 @@ func (rp *Repo) RepoLog(w http.ResponseWriter, r *http.Request) { 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 @@ func (rp *Repo) RepoLog(w http.ResponseWriter, r *http.Request) { } 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 @@ func (rp *Repo) RepoLog(w http.ResponseWriter, r *http.Request) { } 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 @@ func (rp *Repo) RepoCommit(w http.ResponseWriter, r *http.Request) { 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 @@ func (rp *Repo) RepoCommit(w http.ResponseWriter, r *http.Request) { 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 @@ func (rp *Repo) RepoTree(w http.ResponseWriter, r *http.Request) { } 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 @@ func (rp *Repo) RepoTree(w http.ResponseWriter, r *http.Request) { 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 @@ func (rp *Repo) RepoTree(w http.ResponseWriter, r *http.Request) { // 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 @@ func (rp *Repo) RepoTags(w http.ResponseWriter, r *http.Request) { 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 @@ func (rp *Repo) RepoBranches(w http.ResponseWriter, r *http.Request) { 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 @@ func (rp *Repo) RepoBlob(w http.ResponseWriter, r *http.Request) { } 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 @@ func (rp *Repo) RepoBlob(w http.ResponseWriter, r *http.Request) { 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 @@ func (rp *Repo) RepoBlob(w http.ResponseWriter, r *http.Request) { // 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 @@ func (rp *Repo) RepoBlobRaw(w http.ResponseWriter, r *http.Request) { } 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 @@ func (rp *Repo) RepoBlobRaw(w http.ResponseWriter, r *http.Request) { } 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 @@ func (rp *Repo) generalSettings(w http.ResponseWriter, r *http.Request) { 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) pipelineSettings(w http.ResponseWriter, r *http.Request) { 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 @@ func (rp *Repo) RepoCompareNew(w http.ResponseWriter, r *http.Request) { 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 @@ func (rp *Repo) RepoCompareNew(w http.ResponseWriter, r *http.Request) { } 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 @@ func (rp *Repo) RepoCompare(w http.ResponseWriter, r *http.Request) { 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 @@ func (rp *Repo) RepoCompare(w http.ResponseWriter, r *http.Request) { } 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 @@ func (rp *Repo) RepoCompare(w http.ResponseWriter, r *http.Request) { } 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 index 88071d98..9550ceb0 100644 --- a/knotserver/xrpc/repo_archive.go +++ b/knotserver/xrpc/repo_archive.go @@ -13,12 +13,16 @@ import ( ) 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 == "" { format = "tar.gz" // default @@ -34,7 +38,7 @@ func (x *Xrpc) RepoArchive(w http.ResponseWriter, r *http.Request) { 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 @@ func (x *Xrpc) RepoArchive(w http.ResponseWriter, r *http.Request) { 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 index ecfbadb3..9769282e 100644 --- a/knotserver/xrpc/repo_blob.go +++ b/knotserver/xrpc/repo_blob.go @@ -16,12 +16,16 @@ import ( ) 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 == "" { writeError(w, xrpcerr.NewXrpcError( diff --git a/knotserver/xrpc/repo_branch.go b/knotserver/xrpc/repo_branch.go index e4b8db9c..18828733 100644 --- a/knotserver/xrpc/repo_branch.go +++ b/knotserver/xrpc/repo_branch.go @@ -4,6 +4,7 @@ import ( "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 @@ func (x *Xrpc) RepoBranch(w http.ResponseWriter, r *http.Request) { 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 @@ func (x *Xrpc) RepoBranch(w http.ResponseWriter, r *http.Request) { 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 index 2d435496..1e3e748f 100644 --- a/knotserver/xrpc/repo_branches.go +++ b/knotserver/xrpc/repo_branches.go @@ -47,10 +47,7 @@ func (x *Xrpc) RepoBranches(w http.ResponseWriter, r *http.Request) { } } - 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 index 85ab5256..e08d8aa4 100644 --- a/knotserver/xrpc/repo_compare.go +++ b/knotserver/xrpc/repo_compare.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "net/http" - "net/url" "tangled.sh/tangled.sh/core/knotserver/git" "tangled.sh/tangled.sh/core/types" @@ -19,8 +18,8 @@ func (x *Xrpc) RepoCompare(w http.ResponseWriter, r *http.Request) { 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,8 +27,8 @@ func (x *Xrpc) RepoCompare(w http.ResponseWriter, r *http.Request) { 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"), @@ -37,9 +36,6 @@ func (x *Xrpc) RepoCompare(w http.ResponseWriter, r *http.Request) { return } - rev1, _ := url.PathUnescape(rev1Param) - rev2, _ := url.PathUnescape(rev2Param) - gr, err := git.PlainOpen(repoPath) if err != nil { writeError(w, xrpcerr.NewXrpcError( diff --git a/knotserver/xrpc/repo_diff.go b/knotserver/xrpc/repo_diff.go index 3c0510ca..0219bc76 100644 --- a/knotserver/xrpc/repo_diff.go +++ b/knotserver/xrpc/repo_diff.go @@ -3,7 +3,6 @@ package xrpc import ( "encoding/json" "net/http" - "net/url" "tangled.sh/tangled.sh/core/knotserver/git" "tangled.sh/tangled.sh/core/types" @@ -18,16 +17,8 @@ func (x *Xrpc) RepoDiff(w http.ResponseWriter, r *http.Request) { 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 index 419a4e9f..4ae67aa3 100644 --- a/knotserver/xrpc/repo_get_default_branch.go +++ b/knotserver/xrpc/repo_get_default_branch.go @@ -3,6 +3,7 @@ package xrpc import ( "encoding/json" "net/http" + "time" "tangled.sh/tangled.sh/core/api/tangled" "tangled.sh/tangled.sh/core/knotserver/git" @@ -17,14 +18,7 @@ func (x *Xrpc) RepoGetDefaultBranch(w http.ResponseWriter, r *http.Request) { 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 @@ func (x *Xrpc) RepoGetDefaultBranch(w http.ResponseWriter, r *http.Request) { 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 index feba81be..d63cbf8d 100644 --- a/knotserver/xrpc/repo_languages.go +++ b/knotserver/xrpc/repo_languages.go @@ -5,7 +5,6 @@ import ( "encoding/json" "math" "net/http" - "net/url" "time" "tangled.sh/tangled.sh/core/api/tangled" @@ -14,12 +13,6 @@ import ( ) 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 { @@ -27,6 +20,8 @@ func (x *Xrpc) RepoLanguages(w http.ResponseWriter, r *http.Request) { return } + ref := r.URL.Query().Get("ref") + gr, err := git.Open(repoPath, ref) if err != nil { x.Logger.Error("opening repo", "error", err.Error()) diff --git a/knotserver/xrpc/repo_log.go b/knotserver/xrpc/repo_log.go index fcff1f0b..cde39485 100644 --- a/knotserver/xrpc/repo_log.go +++ b/knotserver/xrpc/repo_log.go @@ -3,7 +3,6 @@ package xrpc import ( "encoding/json" "net/http" - "net/url" "strconv" "tangled.sh/tangled.sh/core/knotserver/git" @@ -19,14 +18,7 @@ func (x *Xrpc) RepoLog(w http.ResponseWriter, r *http.Request) { 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") @@ -38,15 +30,6 @@ func (x *Xrpc) RepoLog(w http.ResponseWriter, r *http.Request) { } } - 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 { writeError(w, xrpcerr.NewXrpcError( diff --git a/knotserver/xrpc/repo_tags.go b/knotserver/xrpc/repo_tags.go index 1d20036f..5eeec576 100644 --- a/knotserver/xrpc/repo_tags.go +++ b/knotserver/xrpc/repo_tags.go @@ -30,13 +30,13 @@ func (x *Xrpc) RepoTags(w http.ResponseWriter, r *http.Request) { } } - 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 index 2dc8c0ae..20df113c 100644 --- a/knotserver/xrpc/repo_tree.go +++ b/knotserver/xrpc/repo_tree.go @@ -3,8 +3,8 @@ package xrpc 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,27 +21,12 @@ func (x *Xrpc) RepoTree(w http.ResponseWriter, r *http.Request) { 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 { x.Logger.Error("failed to open git repository", "error", err, "path", repoPath, "ref", ref) @@ -77,7 +62,7 @@ func (x *Xrpc) RepoTree(w http.ResponseWriter, r *http.Request) { 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 index c25d2f5e..ea2451bc 100644 --- a/knotserver/xrpc/xrpc.go +++ b/knotserver/xrpc/xrpc.go @@ -4,7 +4,6 @@ import ( "encoding/json" "log/slog" "net/http" - "net/url" "strings" securejoin "github.com/cyphar/filepath-securejoin" @@ -88,16 +87,16 @@ func (x *Xrpc) parseRepoParam(repo string) (string, error) { } // 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) @@ -119,28 +118,6 @@ func (x *Xrpc) parseRepoParam(repo string) (string, error) { 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) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(status) -- 2.43.0