fix: consider only non-empty values when parsing pagination parameters from URLs

alnr · alnr · commit 1c6366eee66a · 2024-01-04T12:08:01.000+01:00
diff --git a/pagination/keysetpagination/header.go b/pagination/keysetpagination/header.go
@@ -11,6 +11,8 @@ import (
 	"strings"
 
 	"github.com/pkg/errors"
+
+	"github.com/ory/x/stringsx"
 )
 
 // Pagination Request Parameters
@@ -94,8 +96,8 @@ func Header(w http.ResponseWriter, u *url.URL, p *Paginator) {
 // Parse returns the pagination options from the URL query.
 func Parse(q url.Values, p PageTokenConstructor) ([]Option, error) {
 	var opts []Option
-	if q.Has("page_token") {
-		pageToken, err := url.QueryUnescape(q.Get("page_token"))
+	if pt := stringsx.Coalesce(q["page_token"]...); pt != "" {
+		pageToken, err := url.QueryUnescape(pt)
 		if err != nil {
 			return nil, errors.WithStack(err)
 		}
@@ -105,8 +107,8 @@ func Parse(q url.Values, p PageTokenConstructor) ([]Option, error) {
 		}
 		opts = append(opts, WithToken(parsed))
 	}
-	if q.Get("page_size") != "" {
-		size, err := strconv.Atoi(q.Get("page_size"))
+	if ps := stringsx.Coalesce(q["page_size"]...); ps != "" {
+		size, err := strconv.Atoi(ps)
 		if err != nil {
 			return nil, errors.WithStack(err)
 		}
diff --git a/pagination/keysetpagination/paginator_test.go b/pagination/keysetpagination/paginator_test.go
@@ -168,6 +168,26 @@ func TestParse(t *testing.T) {
 		_, err := Parse(url.Values{"page_size": {"invalid-int"}}, NewStringPageToken)
 		require.ErrorIs(t, err, strconv.ErrSyntax)
 	})
+
+	t.Run("empty tokens and page sizes work as if unset, empty values are skipped", func(t *testing.T) {
+		opts, err := Parse(url.Values{}, NewStringPageToken)
+		require.NoError(t, err)
+		paginator := GetPaginator(append(opts, WithDefaultToken(StringPageToken("default")))...)
+		assert.Equal(t, "default", paginator.Token().Encode())
+		assert.Equal(t, 100, paginator.Size())
+
+		opts, err = Parse(url.Values{"page_token": {""}, "page_size": {""}}, NewStringPageToken)
+		require.NoError(t, err)
+		paginator = GetPaginator(append(opts, WithDefaultToken(StringPageToken("default2")))...)
+		assert.Equal(t, "default2", paginator.Token().Encode())
+		assert.Equal(t, 100, paginator.Size())
+
+		opts, err = Parse(url.Values{"page_token": {"", "foo", ""}, "page_size": {"", "123", ""}}, NewStringPageToken)
+		require.NoError(t, err)
+		paginator = GetPaginator(append(opts, WithDefaultToken(StringPageToken("default3")))...)
+		assert.Equal(t, "foo", paginator.Token().Encode())
+		assert.Equal(t, 123, paginator.Size())
+	})
 }
 
 func TestPaginateWithAdditionalColumn(t *testing.T) {

Original file line number	Diff line number	Diff line change
`@@ -11,6 +11,8 @@ import (`
`11`	`11`	`"strings"`
`12`	`12`
`13`	`13`	`"github.com/pkg/errors"`
	`14`	`+`
	`15`	`+ "github.com/ory/x/stringsx"`
`14`	`16`	`)`
`15`	`17`
`16`	`18`	`// Pagination Request Parameters`
`@@ -94,8 +96,8 @@ func Header(w http.ResponseWriter, u url.URL, p Paginator) {`
`94`	`96`	`// Parse returns the pagination options from the URL query.`
`95`	`97`	`func Parse(q url.Values, p PageTokenConstructor) ([]Option, error) {`
`96`	`98`	`var opts []Option`
`97`		`- if q.Has("page_token") {`
`98`		`- pageToken, err := url.QueryUnescape(q.Get("page_token"))`
	`99`	`+ if pt := stringsx.Coalesce(q["page_token"]...); pt != "" {`
	`100`	`+ pageToken, err := url.QueryUnescape(pt)`
`99`	`101`	`if err != nil {`
`100`	`102`	`return nil, errors.WithStack(err)`
`101`	`103`	`}`
`@@ -105,8 +107,8 @@ func Parse(q url.Values, p PageTokenConstructor) ([]Option, error) {`
`105`	`107`	`}`
`106`	`108`	`opts = append(opts, WithToken(parsed))`
`107`	`109`	`}`
`108`		`- if q.Get("page_size") != "" {`
`109`		`- size, err := strconv.Atoi(q.Get("page_size"))`
	`110`	`+ if ps := stringsx.Coalesce(q["page_size"]...); ps != "" {`
	`111`	`+ size, err := strconv.Atoi(ps)`
`110`	`112`	`if err != nil {`
`111`	`113`	`return nil, errors.WithStack(err)`
`112`	`114`	`}`