diff --git a/apps/api/internal/httpapi/features.go b/apps/api/internal/httpapi/features.go index e812aaaf..79cddb72 100644 --- a/apps/api/internal/httpapi/features.go +++ b/apps/api/internal/httpapi/features.go @@ -127,14 +127,14 @@ func parseSearchPageRequest(r *http.Request, userID string) (store.SearchPageReq values := r.URL.Query() limit := 0 if rawLimit := strings.TrimSpace(values.Get("limit")); rawLimit != "" { - parsed, err := strconv.Atoi(rawLimit) + parsed, err := strconv.ParseInt(rawLimit, 10, 32) if err != nil { return store.SearchPageRequest{}, fmt.Errorf("%w: limit must be an integer", store.ErrInvalidSearch) } if parsed <= 0 { return store.SearchPageRequest{}, fmt.Errorf("%w: limit must be positive", store.ErrInvalidSearch) } - limit = parsed + limit = int(parsed) } return store.SearchPageRequest{ WorkspaceID: values.Get("workspace_id"), diff --git a/apps/api/internal/httpapi/server.go b/apps/api/internal/httpapi/server.go index afcb4f2c..79947cff 100644 --- a/apps/api/internal/httpapi/server.go +++ b/apps/api/internal/httpapi/server.go @@ -763,11 +763,11 @@ func parseWorkspaceMemberPageRequest(r *http.Request) (store.WorkspaceMemberPage Role: values.Get("role"), } if rawLimit := strings.TrimSpace(values.Get("limit")); rawLimit != "" { - limit, err := strconv.Atoi(rawLimit) + limit, err := strconv.ParseInt(rawLimit, 10, 32) if err != nil || limit < 1 { return page, fmt.Errorf("%w: limit must be positive", store.ErrInvalidWorkspaceMemberPage) } - page.Limit = limit + page.Limit = int(limit) } return page, nil } @@ -1845,11 +1845,11 @@ func optionalString(value string) *string { } func queryInt(r *http.Request, key string, fallback int) int { - value, err := strconv.Atoi(r.URL.Query().Get(key)) + value, err := strconv.ParseInt(r.URL.Query().Get(key), 10, 32) if err != nil { return fallback } - return value + return int(value) } func parseMessagePageRequest(r *http.Request) (store.MessagePageRequest, error) { diff --git a/apps/api/internal/httpapi/server_test.go b/apps/api/internal/httpapi/server_test.go index 60662565..bfe5f5fc 100644 --- a/apps/api/internal/httpapi/server_test.go +++ b/apps/api/internal/httpapi/server_test.go @@ -3711,9 +3711,28 @@ func TestUploadRejectsInvalidMultipartShapes(t *testing.T) { func TestQueryHelpersParseValues(t *testing.T) { t.Parallel() - req := httptest.NewRequest(http.MethodGet, "/?limit=42", nil) - if got := queryInt(req, "limit", 10); got != 42 { - t.Fatalf("unexpected int query value %d", got) + for _, test := range []struct { + name string + value string + want int + }{ + {name: "missing", want: 10}, + {name: "invalid", value: "bad", want: 10}, + {name: "minimum database integer", value: "-2147483648", want: -2147483648}, + {name: "negative", value: "-1", want: -1}, + {name: "zero", value: "0", want: 0}, + {name: "positive", value: "42", want: 42}, + {name: "maximum database integer", value: "2147483647", want: 2147483647}, + {name: "above database integer", value: "2147483648", want: 10}, + {name: "below database integer", value: "-2147483649", want: 10}, + {name: "maximum 64-bit integer", value: "9223372036854775807", want: 10}, + } { + t.Run(test.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/?limit="+test.value, nil) + if got := queryInt(req, "limit", 10); got != test.want { + t.Fatalf("queryInt() = %d, want %d", got, test.want) + } + }) } server := New(nil, nil, Options{GitHubOAuth: GitHubOAuthConfig{PublicURL: "https://app.clickclack.test/path"}}) patterns := server.websocketOriginPatterns(httptest.NewRequest(http.MethodGet, "/", nil)) @@ -3725,6 +3744,44 @@ func TestQueryHelpersParseValues(t *testing.T) { } } +func TestPaginationLimitParsersUseDatabaseWidth(t *testing.T) { + t.Parallel() + for _, test := range []struct { + name string + value string + want int + wantErr bool + }{ + {name: "missing"}, + {name: "negative", value: "-1", wantErr: true}, + {name: "zero", value: "0", wantErr: true}, + {name: "positive", value: "1", want: 1}, + {name: "maximum database integer", value: "2147483647", want: 2147483647}, + {name: "above database integer", value: "2147483648", wantErr: true}, + {name: "maximum 64-bit integer", value: "9223372036854775807", wantErr: true}, + } { + t.Run(test.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/?limit="+test.value, nil) + + search, searchErr := parseSearchPageRequest(req, "usr_test") + if got := searchErr != nil; got != test.wantErr { + t.Fatalf("parseSearchPageRequest() error = %v, want error %v", searchErr, test.wantErr) + } + if searchErr == nil && search.Limit != test.want { + t.Fatalf("search limit = %d, want %d", search.Limit, test.want) + } + + members, memberErr := parseWorkspaceMemberPageRequest(req) + if got := memberErr != nil; got != test.wantErr { + t.Fatalf("parseWorkspaceMemberPageRequest() error = %v, want error %v", memberErr, test.wantErr) + } + if memberErr == nil && members.Limit != test.want { + t.Fatalf("member limit = %d, want %d", members.Limit, test.want) + } + }) + } +} + func TestDirectRealtimeEventsRespectGuestDemotion(t *testing.T) { t.Parallel() ctx := context.Background() diff --git a/apps/api/internal/store/postgres/members.go b/apps/api/internal/store/postgres/members.go index 9396e8ca..8aad7a4a 100644 --- a/apps/api/internal/store/postgres/members.go +++ b/apps/api/internal/store/postgres/members.go @@ -211,7 +211,7 @@ func postgresWorkspaceMemberCounts(ctx context.Context, tx *sql.Tx, workspaceID func postgresWorkspaceMemberPageFromRows(workspaceID string, req store.WorkspaceMemberPageRequest, totalCount *int, totalByRole *store.WorkspaceMemberRoleCounts, rows []storedb.ListWorkspaceMemberPageRow) (store.WorkspaceMemberPage, error) { page := store.WorkspaceMemberPage{ - Members: make([]store.WorkspaceMember, 0, min(len(rows), req.Limit)), + Members: make([]store.WorkspaceMember, 0, len(rows)), TotalCount: totalCount, TotalByRole: totalByRole, } diff --git a/apps/api/internal/store/search_pages.go b/apps/api/internal/store/search_pages.go index 67ab74a8..402d0364 100644 --- a/apps/api/internal/store/search_pages.go +++ b/apps/api/internal/store/search_pages.go @@ -155,7 +155,7 @@ func EncodeSearchCursor(req SearchPageRequest, rank float64, createdAt, messageI func BuildSearchPage(req SearchPageRequest, entries []SearchPageEntry) (SearchPage, error) { page := SearchPage{ - Results: make([]SearchHit, 0, min(len(entries), req.Limit)), + Results: make([]SearchHit, 0, len(entries)), } hasMore := len(entries) > req.Limit if hasMore { diff --git a/apps/api/internal/store/sqlite/members.go b/apps/api/internal/store/sqlite/members.go index 3b308bb0..9a2f38ea 100644 --- a/apps/api/internal/store/sqlite/members.go +++ b/apps/api/internal/store/sqlite/members.go @@ -212,7 +212,7 @@ func sqliteWorkspaceMemberCounts(ctx context.Context, tx *sql.Tx, workspaceID st func sqliteWorkspaceMemberPageFromRows(workspaceID string, req store.WorkspaceMemberPageRequest, totalCount *int, totalByRole *store.WorkspaceMemberRoleCounts, rows []storedb.ListWorkspaceMemberPageRow) (store.WorkspaceMemberPage, error) { page := store.WorkspaceMemberPage{ - Members: make([]store.WorkspaceMember, 0, min(len(rows), req.Limit)), + Members: make([]store.WorkspaceMember, 0, len(rows)), TotalCount: totalCount, TotalByRole: totalByRole, }