Handle corner cases for pagination of list rules

Signed-off-by: Raphael Silva <[email protected]>
qinxx108 · Oct 8, 2024 · dc47970 · dc47970
1 parent 2ef6532
commit dc47970
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 16 deletions.
diff --git a/web/api/v1/api.go b/web/api/v1/api.go
@@ -1590,7 +1590,7 @@ func (api *API) rules(r *http.Request) apiFuncResult {
 
 	if paginationRequest != nil && paginationRequest.NextToken != "" && !foundToken {
 		err := fmt.Errorf("invalid nextToken '%v'. were rule groups changed?", paginationRequest.NextToken)
-		return apiFuncResult{nil, &apiError{errorBadData, err}, nil, nil}
+		return invalidParamError(err, "next_token")
 	}
 
 	res.RuleGroups = rgs
@@ -1613,28 +1613,32 @@ func parseExcludeAlerts(r *http.Request) (bool, error) {
 
 func parseListRulesPaginationRequest(r *http.Request) (*listRulesPaginationRequest, *parsePaginationError) {
 	var (
-		maxRuleGroups int64 = -1
-		nextToken           = ""
-		err           error
+		parsedMaxGroups int64 = -1
+		err             error
 	)
+	maxGroups := r.URL.Query().Get("max_groups")
+	nextToken := r.URL.Query().Get("next_token")
+
+	if nextToken != "" && maxGroups == "" {
+		return nil, &parsePaginationError{
+			err:       fmt.Errorf("max_groups needs to be present in order to paginate"),
+			parameter: "max_groups",
+		}
+	}
 
-	if r.URL.Query().Get("max_groups") != "" {
-		maxRuleGroups, err = strconv.ParseInt(r.URL.Query().Get("max_groups"), 10, 32)
-		if err != nil || maxRuleGroups < 0 {
+	if maxGroups != "" {
+		parsedMaxGroups, err = strconv.ParseInt(maxGroups, 10, 32)
+		if err != nil || parsedMaxGroups <= 0 {
 			return nil, &parsePaginationError{
-				err:       fmt.Errorf("max_groups need to be a valid number greater than or equal to 0: %w", err),
+				err:       fmt.Errorf("max_groups need to be a valid number greater than 0: %w", err),
 				parameter: "max_groups",
 			}
 		}
 	}
 
-	if r.URL.Query().Get("next_token") != "" {
-		nextToken = r.URL.Query().Get("next_token")
-	}
-
-	if maxRuleGroups >= 0 || nextToken != "" {
+	if parsedMaxGroups >= 0 || nextToken != "" {
 		return &listRulesPaginationRequest{
-			MaxRuleGroups: maxRuleGroups,
+			MaxRuleGroups: parsedMaxGroups,
 			NextToken:     nextToken,
 		}, nil
 	}

diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go
@@ -2874,11 +2874,28 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
 			},
 			zeroFunc: rulesZeroFunc,
 		},
-		{
+		{ // invalid pagination request
+			endpoint: api.rules,
+			query: url.Values{
+				"next_token": []string{getRuleGroupNextToken("/path/to/file", "grp2")},
+			},
+			errType:  errorBadData,
+			zeroFunc: rulesZeroFunc,
+		},
+		{ // invalid max_groups
+			endpoint: api.rules,
+			query: url.Values{
+				"max_groups": []string{"0"},
+				"next_token": []string{getRuleGroupNextToken("/path/to/file", "grp2")},
+			},
+			errType:  errorBadData,
+			zeroFunc: rulesZeroFunc,
+		},
+		{ // Pagination token is invalid due to changes in the rule groups
 			endpoint: api.rules,
 			query: url.Values{
 				"max_groups": []string{"1"},
-				"next_token": []string{getRuleGroupNextToken("/path/to/not/exist", "unknown")},
+				"next_token": []string{getRuleGroupNextToken("/removed/file", "notfound")},
 			},
 			errType:  errorBadData,
 			zeroFunc: rulesZeroFunc,