Skip to content

Reject invalid ArrayConnection cursors - #5715

Merged
rmosolgo merged 1 commit into
rmosolgo:masterfrom
ydah:reject-invalid-array-connection-cursors
Aug 29, 2026
Merged

Reject invalid ArrayConnection cursors#5715
rmosolgo merged 1 commit into
rmosolgo:masterfrom
ydah:reject-invalid-array-connection-cursors

Conversation

@ydah

@ydah ydah commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

GraphQL::Pagination::ArrayConnection currently converts decoded cursors with String#to_i and uses the result directly as an Array index.

This allows negative cursor offsets to wrap around to the end of the Array:

items = (1..10).map { |i| "item#{i}" }
after: "3"  # => ["item4", "item5", "item6"]
after: "-3" # => ["item8", "item9", "item10"]
after: "-1" # => ["item10"]

Malformed values are also partially or silently converted:

"abc".to_i  # => 0
"1e10".to_i # => 1

As a result, invalid client-provided cursors can return unrelated pages and produce incorrect pageInfo values.

This PR parses decoded ArrayConnection cursors with Integer and raises GraphQL::ExecutionError for malformed, zero, or negative values. Positive offsets beyond the Array continue to return an empty page, preserving the existing out-of-bounds behavior. Very large positive integers are capped to a safe sentinel index so they cannot raise a Ruby RangeError.

if index.nil? || index <= 0
raise GraphQL::ExecutionError, "Invalid cursor: #{cursor.inspect}"
end
[index, items.length + 1].min

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to make sure I understand this right:

If you have a (stale) cursor from previously fetching this Array, and the Array is now shorter than it used to be (so that your stale cursor is now out-of-bounds), will the code still return an empty result set, or will it raise an error?

(I think it will return an empty result set, which is the correct behavior per https://relay.dev/graphql/connections.htm#sec-Pagination-algorithm -- I just want to make sure I understand correctly!)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's correct. Positive out-of-bounds cursors are still accepted. The index is capped at items.length + 1, so an after cursor beyond the current Array slices past the end and returns an empty result set. Only malformed, zero, or negative cursors raise an error.

The existing “handles out-of-bounds cursors” test covers this with an encoded cursor value of "100", which is equivalent to a stale cursor from a previously longer Array. Thanks for checking!

@ydah
ydah force-pushed the reject-invalid-array-connection-cursors branch from 387dbea to 940040c Compare August 28, 2026 13:40
@rmosolgo rmosolgo added this to the 2.6.11 milestone Aug 29, 2026
@rmosolgo

Copy link
Copy Markdown
Owner

Sounds good, thanks for this!

@rmosolgo
rmosolgo merged commit 617cac0 into rmosolgo:master Aug 29, 2026
11 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants