fix: a huge export or list page answers an empty page instead of panicking (WR-14)
(page-1)*perPage overflowed for a large page and sliced out of range. Export compares the page with the page count before multiplying, and the list endpoints cap the page number.
This commit is contained in:
@@ -160,9 +160,11 @@ func (s *Service) Export(ctx context.Context, dataset, delimiter string, page, p
|
||||
page = 1
|
||||
}
|
||||
out.Page = page
|
||||
start := (page - 1) * perPage
|
||||
if start > len(rows) {
|
||||
start = len(rows)
|
||||
// A page past the last is empty; compared before multiplying, so a
|
||||
// huge page cannot overflow into a negative start (WR-14).
|
||||
start := len(rows)
|
||||
if page <= out.Pages {
|
||||
start = (page - 1) * perPage
|
||||
}
|
||||
end := min(start+perPage, len(rows))
|
||||
rows = rows[start:end]
|
||||
|
||||
@@ -184,10 +184,12 @@ func read(ctx context.Context, svc *classes.Service) (*gorm.DB, error) {
|
||||
return svc.DB(ctx)
|
||||
}
|
||||
|
||||
// The page sizes of the list endpoints.
|
||||
// The page sizes of the list endpoints, and the highest page number read
|
||||
// (a later page is empty anyway).
|
||||
const (
|
||||
defaultPerPage = 50
|
||||
maxPerPage = 200
|
||||
maxPage = 1_000_000
|
||||
)
|
||||
|
||||
// ListInput is the paging and search query every list tool shares.
|
||||
@@ -206,6 +208,10 @@ func listParams(r *http.Request) ListInput {
|
||||
if in.Page < 1 {
|
||||
in.Page = 1
|
||||
}
|
||||
// (Page-1)*PerPage must not overflow into a negative offset (WR-14).
|
||||
if in.Page > maxPage {
|
||||
in.Page = maxPage
|
||||
}
|
||||
if in.PerPage < 1 {
|
||||
in.PerPage = defaultPerPage
|
||||
}
|
||||
|
||||
@@ -73,6 +73,16 @@ func TestCSVTransfer(t *testing.T) {
|
||||
t.Fatalf("filename = %v", exp["filename"])
|
||||
}
|
||||
|
||||
// A huge page number is an empty page, not a panic (WR-14); so is a
|
||||
// list page.
|
||||
far := env.mcpExpect(t, http.StatusOK, http.MethodGet, "/_seo/api/v1/export?dataset=redirects&page=9223372036854777", agent, nil)
|
||||
if c, _ := far["csv"].(string); strings.Contains(c, "/e1") {
|
||||
t.Fatalf("export of a huge page = %v, want no rows", far)
|
||||
}
|
||||
if l := env.mcpExpect(t, http.StatusOK, http.MethodGet, "/_seo/api/v1/redirects?page=9223372036854775807&per_page=200", agent, nil); len(l["data"].([]any)) != 0 {
|
||||
t.Fatalf("list of a huge page = %v, want no rows", l)
|
||||
}
|
||||
|
||||
preview := env.mcpExpect(t, http.StatusOK, http.MethodPost, "/_seo/api/v1/import", agent, map[string]any{"dataset": "redirects", "csv": csv, "delimiter": ";", "dry_run": true})
|
||||
if s := summary(t, preview); s["unchanged"] != 2 || s["new"] != 0 || s["changed"] != 0 || s["refused"] != 0 {
|
||||
t.Fatalf("re-import of the export = %v, want 2 unchanged", preview)
|
||||
|
||||
Reference in New Issue
Block a user