Skip to content

[2.x] Fix 500 when page[limit]=0 is requested#4775

Open
linkrobins wants to merge 1 commit into
flarum:2.xfrom
linkrobins:fix/page-limit-zero-500
Open

[2.x] Fix 500 when page[limit]=0 is requested#4775
linkrobins wants to merge 1 commit into
flarum:2.xfrom
linkrobins:fix/page-limit-zero-500

Conversation

@linkrobins

Copy link
Copy Markdown
Contributor

Fixes #4773.

Problem

Any request to an offset-paginated endpoint with page[limit]=0 returns a 500:

GET /api/discussions?page[limit]=0

TypeError: Cannot assign null to property
Tobyz\JsonApiServer\Pagination\OffsetPagination::$limit of type int
in core/src/Api/Endpoint/Index.php:133

This is what was recorded on discuss in #4773. It reproduces on a clean install with no extensions.

Cause

RequestUtil::extractLimit() short-circuits with null on a falsy limit:

if (! $limit) {
    return null;
}

The string "0" is falsy, so it returns null before reaching the < 1 validation just below. That null is then assigned to OffsetPagination::$limit, which is a non-nullable int, so PHP throws.

The boundary confirms the mechanism — only the literal 0 hits this. -5, 00, 0.0 are all rejected with a clean 400, because they are truthy strings that reach the < 1 guard.

Fix

Use a strict === null check. A genuinely absent limit (no page[limit] and no default) still returns null, but an explicit page[limit]=0 now falls through to the existing < 1 guard and returns a 400, consistent with how negative limits are already handled.

Added unit tests in RequestUtilTest covering an explicit limit, capping at max, the default fallback, the no-limit null path, and rejection of 0 and negative values.

`RequestUtil::extractLimit()` used a loose `! $limit` check that also
matched the string "0", returning null. That null was then assigned to
`OffsetPagination`'s non-nullable `int $limit`, throwing a TypeError and
producing a 500 on any offset-paginated endpoint.

Use a strict `=== null` check so a genuinely absent limit still returns
null (the "no limit" path), while an explicit `page[limit]=0` falls
through to the existing `< 1` guard and returns a 400, consistent with
how negative limits are already handled.

Fixes flarum#4773
@linkrobins linkrobins requested a review from a team as a code owner June 20, 2026 00:55
@linkrobins linkrobins mentioned this pull request Jun 20, 2026
@linkrobins linkrobins changed the title Fix 500 when page[limit]=0 is requested [2.x] Fix 500 when page[limit]=0 is requested Jun 20, 2026
@imorland

Copy link
Copy Markdown
Member

All well and good, but what is actually calling page[limit]=0 in the first place? If we don't know that, then as sensible as this patch might seem, we still have potentially a useless API call being triggered from somewhere. This needs investigating deeper before we can move forward with this one

@linkrobins

Copy link
Copy Markdown
Contributor Author

All well and good, but what is actually calling page[limit]=0 in the first place? If we don't know that, then as sensible as this patch might seem, we still have potentially a useless API call being triggered from somewhere. This needs investigating deeper before we can move forward with this one

I'll look at this again and aid in pinpointing the origin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[2.x] API Error

2 participants