Handle (theoretical) cursor encoding corner case

ogc/features/domain/cursor.go

@@ -49,10 +49,10 @@ func NewCursors(fid PrevNextFID, filtersChecksum []byte) Cursors {
 func encodeCursor(fid int64, filtersChecksum []byte) EncodedCursor {
 	fidAsBytes := big.NewInt(fid).Bytes()
 
-	// format of the cursor: <fid><separator><hash>
+	// format of the cursor: <fid><separator><checksum>
 	cursor := fidAsBytes
 	cursor = append(cursor, byte(separator))
-	cursor = append(cursor, filtersChecksum...)
+	cursor = append(cursor, filtersChecksum...) // could contain any byte, so always keep this as the last element
 
 	return EncodedCursor(base64.URLEncoding.EncodeToString(cursor))
 }
@@ -71,24 +71,21 @@ func (c EncodedCursor) Decode(filtersChecksum []byte) DecodedCursor {
 		return DecodedCursor{0, filtersChecksum}
 	}
 
-	decodedParts := bytes.Split(decoded, []byte{separator})
-	if len(decoded) < 1 {
+	decodedFid, decodedChecksum, found := bytes.Cut(decoded, []byte{separator})
+	if !found {
+		log.Printf("cursor '%v' doesn't contain expected separator %c", decoded, separator)
 		return DecodedCursor{0, filtersChecksum}
 	}
 
 	// feature id
-	fid := big.NewInt(0).SetBytes(decodedParts[0]).Int64()
-	if err != nil {
-		log.Printf("cursor %s doesn't contain numeric value, defaulting to first page", decodedParts[0])
-		return DecodedCursor{0, filtersChecksum}
-	}
+	fid := big.NewInt(0).SetBytes(decodedFid).Int64()
 	if fid < 0 {
 		log.Printf("negative feature ID detected: %d, defaulting to first page", fid)
 		fid = 0
 	}
 
 	// checksum
-	if len(decodedParts) > 1 && !bytes.Equal(decodedParts[1], filtersChecksum) {
+	if !bytes.Equal(decodedChecksum, filtersChecksum) {
 		log.Printf("filters in query params have changed during pagination, resetting to first page")
 		return DecodedCursor{0, filtersChecksum}
 	}

ogc/features/domain/cursor_test.go

@@ -1,6 +1,7 @@
 package domain
 
 import (
+	"math"
 	"reflect"
 	"testing"
 )
@@ -73,3 +74,89 @@ func TestNewCursor(t *testing.T) {
 		})
 	}
 }
+
+func TestEncodedCursor_Decode(t *testing.T) {
+	type args struct {
+		filtersChecksum []byte
+	}
+	tests := []struct {
+		name string
+		c    EncodedCursor
+		args args
+		want DecodedCursor
+	}{
+		{
+			name: "should return cursor if no checksum is available in cursor, and no expected checksum provided",
+			c:    encodeCursor(123, []byte{}),
+			args: args{
+				filtersChecksum: []byte{},
+			},
+			want: DecodedCursor{
+				FID:             123,
+				FiltersChecksum: []byte{},
+			},
+		},
+		{
+			name: "should not fail on checksum which contains separator",
+			c:    encodeCursor(123456, []byte{'a', separator, 'b'}),
+			args: args{
+				filtersChecksum: []byte{'a', separator, 'b'},
+			},
+			want: DecodedCursor{
+				FID:             123456,
+				FiltersChecksum: []byte{'a', separator, 'b'},
+			},
+		},
+		{
+			name: "should not fail on checksum which contains only separator",
+			c:    encodeCursor(123456, []byte{separator}),
+			args: args{
+				filtersChecksum: []byte{separator},
+			},
+			want: DecodedCursor{
+				FID:             123456,
+				FiltersChecksum: []byte{separator},
+			},
+		},
+		{
+			name: "should fail (return 0 fid) on non matching checksums",
+			c:    encodeCursor(123456, []byte("foobarbaz")),
+			args: args{
+				filtersChecksum: []byte("bazbar"),
+			},
+			want: DecodedCursor{
+				FID:             0,
+				FiltersChecksum: []byte("bazbar"),
+			},
+		},
+		{
+			name: "should handle large feature id",
+			c:    encodeCursor(math.MaxInt64, []byte("foobar")),
+			args: args{
+				filtersChecksum: []byte("foobar"),
+			},
+			want: DecodedCursor{
+				FID:             math.MaxInt64,
+				FiltersChecksum: []byte("foobar"),
+			},
+		},
+		{
+			name: "should always return positive feature id",
+			c:    encodeCursor(math.MinInt64, []byte("foobar")),
+			args: args{
+				filtersChecksum: []byte("foobar"),
+			},
+			want: DecodedCursor{
+				FID:             0,
+				FiltersChecksum: []byte("foobar"),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.c.Decode(tt.args.filtersChecksum); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("Decode() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

ogc/features/domain/features.go

@@ -59,7 +59,7 @@ func MapRowsToFeatures(rows *sqlx.Rows, fidColumn string, geomColumn string,
 	}
 
 	firstRow := true
-	var nextPrevID *PrevNextFID
+	var prevNextID *PrevNextFID
 	for rows.Next() {
 		var values []interface{}
 		if values, err = rows.SliceScan(); err != nil {
@@ -71,19 +71,19 @@ func MapRowsToFeatures(rows *sqlx.Rows, fidColumn string, geomColumn string,
 		if err != nil {
 			return result, nil, err
 		} else if firstRow {
-			nextPrevID = np
+			prevNextID = np
 			firstRow = false
 		}
 		result = append(result, feature)
 	}
-	return result, nextPrevID, nil
+	return result, prevNextID, nil
 }
 
 //nolint:cyclop,funlen
 func mapColumnsToFeature(firstRow bool, feature *Feature, columns []string, values []interface{},
 	fidColumn string, geomColumn string, geomMapper func([]byte) (geom.Geometry, error)) (*PrevNextFID, error) {
 
-	nextPrevID := PrevNextFID{}
+	prevNextID := PrevNextFID{}
 	for i, columnName := range columns {
 		columnValue := values[i]
 		if columnValue == nil {
@@ -112,13 +112,13 @@ func mapColumnsToFeature(firstRow bool, feature *Feature, columns []string, valu
 		case "prevfid":
 			// Only the first row in the result set contains the previous feature id
 			if firstRow {
-				nextPrevID.Prev = columnValue.(int64)
+				prevNextID.Prev = columnValue.(int64)
 			}
 
 		case "nextfid":
 			// Only the first row in the result set contains the next feature id
 			if firstRow {
-				nextPrevID.Next = columnValue.(int64)
+				prevNextID.Next = columnValue.(int64)
 			}
 
 		default:
@@ -143,5 +143,5 @@ func mapColumnsToFeature(firstRow bool, feature *Feature, columns []string, valu
 			}
 		}
 	}
-	return &nextPrevID, nil
+	return &prevNextID, nil
 }