~gitpocket/gitpocket

8

fix(api): clamp list offsets, page gist revisions and limit only writes

commit
94889e856cd3784c07f08335a6fa359ea3f1e7a7 browse tree
parent
e3842ceec81f72b2c93afa6d77c9114a1b721e1b
author
~james <354576047+[email protected]>
signature
Signed by: ~james SHA256:KlzJ0P0gvJcnOYpaxD5yCUw52ImHttTKRjVAEnnYnKg
date
2026-11-14 15:10 UTC

6 files changed, +38 −35

forge/actions.go modified

@@ -481,8 +481,11 @@ func (s *Server) mergePull(u User, pull Pull, strategy, title, body string) (str
481481 if s.store.IsPullMirror(repo.ID) {
482482 return "", refuse(403, "This repository is a mirror and read only")
483483 }
484- if !s.store.CanWrite(repo, &u) || pull.State != "open" {
485- return "", refuse(403, "Only the repository owner can merge")
484+ if !s.store.CanWrite(repo, &u) {
485+ return "", refuse(403, "Only people who can push here can merge")
486+ }
487+ if pull.State != "open" {
488+ return "", refuse(409, "This pull request is "+pull.State)
486489 }
487490 if pull.Draft {
488491 return "", refuse(409, "Draft pull requests cannot be merged")

forge/api.go modified

@@ -6,7 +6,7 @@ import (
66 "io"
77 "log"
88 "net/http"
9- "net/url"
9+ "slices"
1010 "strconv"
1111 "strings"
1212 "time"
@@ -131,7 +131,9 @@ func (s *Server) serveAPI(w http.ResponseWriter, r *http.Request) {
131131 continue
132132 }
133133 if rt.Method != method {
134- allowed = append(allowed, rt.Method)
134+ if !slices.Contains(allowed, rt.Method) {
135+ allowed = append(allowed, rt.Method)
136+ }
135137 continue
136138 }
137139 c.args = args
@@ -172,11 +174,7 @@ func matchRoute(pattern, segs []string) (map[string]string, bool) {
172174 if !strings.HasPrefix(segs[i], prefix) || len(segs[i]) == len(prefix) {
173175 return nil, false
174176 }
175- v, err := url.PathUnescape(segs[i][len(prefix):])
176- if err != nil {
177- return nil, false
178- }
179- args[name] = v
177+ args[name] = segs[i][len(prefix):]
180178 }
181179 return args, len(segs) == len(pattern)
182180 }
@@ -265,7 +263,7 @@ func (c *call) permit(need string) bool {
265263 c.fail(403, "Only administrators can do that")
266264 return false
267265 }
268- if need != needUser && !c.s.writes.allow(strconv.FormatInt(c.user.ID, 10)) {
266+ if c.r.Method != "GET" && c.r.Method != "HEAD" && !c.s.writes.allow(strconv.FormatInt(c.user.ID, 10)) {
269267 c.w.Header().Set("Retry-After", "10")
270268 c.fail(429, "Too many changes, slow down")
271269 return false
@@ -338,10 +336,10 @@ func paged[T any](c *call, items []T) {
338336 limit = 30
339337 }
340338 limit = min(limit, 100)
341- from, _ := strconv.Atoi(c.query("from"))
342- from = max(from, 0)
343339 total := len(items)
344- page := items[min(from, total):min(from+limit, total)]
340+ from, _ := strconv.Atoi(c.query("from"))
341+ from = min(max(from, 0), total)
342+ page := items[from:min(from+limit, total)]
345343 if page == nil {
346344 page = []T{}
347345 }
@@ -400,14 +398,7 @@ func (c *call) repo() (Repo, bool) {
400398 return repo, true
401399 }
402400
403-func (c *call) number() (int64, bool) {
404- n, err := strconv.ParseInt(c.arg("number"), 10, 64)
405- if err != nil || n <= 0 {
406- c.fail(404, "Not found")
407- return 0, false
408- }
409- return n, true
410-}
401+func (c *call) number() (int64, bool) { return c.id("number") }
411402
412403 func (c *call) id(name string) (int64, bool) {
413404 n, err := strconv.ParseInt(c.arg(name), 10, 64)

forge/api_test.go modified

@@ -280,6 +280,12 @@ func TestAPI(t *testing.T) {
280280 if v := a.want(200, "GET", "/api/gists/"+id+"/revisions", "", ""); v["total"].(float64) != 2 {
281281 t.Errorf("revisions: %v", v)
282282 }
283+ if v := a.want(200, "GET", "/api/gists/"+id+"/revisions?limit=1", "", ""); v["next"] == nil || len(v["items"].([]any)) != 1 {
284+ t.Errorf("revision paging: %v", v)
285+ }
286+ if v := a.want(200, "GET", "/api/gists?from=9223372036854775807", "", ""); len(v["items"].([]any)) != 0 {
287+ t.Errorf("far page: %v", v)
288+ }
283289 a.want(204, "PUT", "/api/gists/"+id+"/star", bt, "")
284290 a.want(201, "POST", "/api/gists/"+id+"/comments", bt, `{"body":"neat"}`)
285291 a.want(201, "POST", "/api/gists/"+id+"/forks", bt, "")

forge/gistweb.go modified

@@ -877,16 +877,14 @@ func (s *Server) apiGistRevisions(c *call) {
877877 return
878878 }
879879 g := s.gistGit(gist)
880- commits, _ := g.Log(s.gistHead(gist), 1000, "")
881880 patch := c.query("patch") == "1"
882- views := mapped(commits, func(cm Commit) commitView {
881+ c.logPage(g, s.gistHead(gist), "", s.gistRevisionCount(gist), func(cm Commit) commitView {
883882 v := s.commitView(cm)
884883 if files, err := g.Diff(cm.Hash); err == nil {
885884 v.Files = nonNil(fileViews(files, patch))
886885 }
887886 return v
888887 })
889- paged(c, views)
890888 }
891889
892890 func (s *Server) apiStarGist(c *call) {

forge/repos.go modified

@@ -619,6 +619,10 @@ func (s *Server) apiLog(c *call) {
619619 c.fail(404, "No such path")
620620 return
621621 }
622+ c.logPage(g, sha, filePath, -1, s.commitView)
623+}
624+
625+func (c *call) logPage(g Git, sha, filePath string, total int, view func(Commit) commitView) {
622626 limit, err := strconv.Atoi(c.query("limit"))
623627 if err != nil || limit <= 0 {
624628 limit = 30
@@ -633,13 +637,17 @@ func (s *Server) apiLog(c *call) {
633637 }
634638 out := struct {
635639 Items []commitView `json:"items"`
640+ Total *int `json:"total,omitempty"`
636641 Next *string `json:"next"`
637- }{Items: mapped(commits[:min(limit, len(commits))], s.commitView)}
642+ }{Items: mapped(commits[:min(limit, len(commits))], view)}
643+ if total >= 0 {
644+ out.Total = &total
645+ }
638646 if len(commits) > limit {
639647 q := c.r.URL.Query()
640648 q.Set("from", strconv.Itoa(from+limit))
641649 q.Set("limit", strconv.Itoa(limit))
642- next := "https://" + s.host + c.r.URL.Path + "?" + q.Encode()
650+ next := "https://" + c.s.host + c.r.URL.Path + "?" + q.Encode()
643651 out.Next = &next
644652 c.w.Header().Set("Link", "<"+next+`>; rel="next"`)
645653 }
@@ -714,12 +722,10 @@ func (s *Server) apiTree(c *call) {
714722 if !ok {
715723 return
716724 }
717- switch g.Kind(sha, filePath) {
718- case "tree", "":
719- if filePath != "" && g.Kind(sha, filePath) == "" {
720- c.fail(404, "No such path")
721- return
722- }
725+ switch kind := g.Kind(sha, filePath); {
726+ case kind == "" && filePath != "":
727+ c.fail(404, "No such path")
728+ case kind == "tree" || kind == "":
723729 entries, err := g.Tree(sha, filePath)
724730 if err != nil {
725731 c.fail(404, "No such path")
@@ -735,7 +741,7 @@ func (s *Server) apiTree(c *call) {
735741 return
736742 }
737743 out := map[string]any{"ref": ref, "commit": sha, "path": filePath, "kind": entryKind(*e), "mode": e.Mode, "size": e.Size, "sha": e.Hash,
738- "raw": "https://" + s.host + "/api/" + repo.Path() + "/raw/" + ref + "/" + escapePath(filePath)}
744+ "raw": "https://" + s.host + "/api/" + repo.Path() + "/raw/" + escapePath(ref) + "/" + escapePath(filePath)}
739745 if e.Size <= 1<<20 && entryKind(*e) != "submodule" {
740746 data, err := g.Blob(sha, filePath)
741747 if err == nil {

forge/threads.go modified

@@ -656,8 +656,7 @@ func (s *Server) apiAddNote(c *call) {
656656 return
657657 }
658658 notes, _ := s.store.Reviews(pull.ID)
659- newest := slices.MaxFunc(notes, func(a, b Review) int { return int(a.ID - b.ID) })
660- c.reply(201, noteViews([]Review{newest})[0])
659+ c.reply(201, noteViews(notes[len(notes)-1:])[0])
661660 }
662661
663662 func (s *Server) apiPatchNote(c *call) {