limit and skip silently guess at invalid input, and an over-maximum skip returns the same page forever #301

Summary

getPagination() (controllers/utils.js:21) never rejects anything. clampNonNegativeInt() (controllers/utils.js:15) substitutes a fallback for any value it cannot parse, and caps anything above the maximum. Every case below returns 200 with a page the client did not ask for and cannot detect:

limit=0     -> 100      limit=abc   -> 100      limit=10abc -> 10
limit=-5    -> 100      limit=1e3   -> 1        limit=0x10  -> 100
limit=250.7 -> 250      limit=      -> 100

skip=-5     -> 0        skip=abc    -> 0        skip=1e3    -> 1
skip=2.9    -> 2        skip=       -> 0

The most damaging case is skip above the maximum. It is clamped rather than rejected, so every request past 100000 returns the identical page, indefinitely. A client that advances skip by the number of records it received and stops on an empty page never terminates.

The helper is shared, so this covers /query, HEAD /query, /search, /search/phrase, and the two /gog/*InManuscript endpoints in one fix site.

Decision recorded on the parent thread: an over-maximum limit is clamped and reported, not rejected. Clamping an over-large page size is conventional server behavior and rejecting it is the more breaking of the two options; the defect is the silence, not the clamp. An over-maximum skip is rejected, because a repeated page has no valid reading.

Why this matters

The non-terminating loop is our own published example. The pagedQuery function at public/API.html:555 stops only on an empty page and advances by results.length. Run verbatim against the local deployment at the real caps, entered at skip=99800 so the cap is reached cheaply:

req #1: skip=99800  -> 100 docs, first=912509cc
req #2: skip=99900  -> 100 docs, first=4c0eef00
req #3: skip=100000 -> 100 docs, first=4c0ef019
req #4: skip=100100 -> 100 docs, first=4c0ef019
req #5: skip=100200 -> 100 docs, first=4c0ef019
...
GUARD TRIPPED after 8 requests
=> 800 records accumulated, 300 distinct

Without the added guard this does not stop and the accumulator grows without bound. The loop is reachable on production today: {"@type":"oa:Annotation"} — every IIIF 2.1 annotation — still returns a document at skip=100000 on store.rerum.io, and {"@type":"Annotation"} is between 50000 and 100000 and growing toward it.

limit=1e3 is a data-loss trap. Number.parseInt("1e3", 10) is 1, so a client asking for a thousand records receives one. A client that stops on a short page reports a completed walk of a single object. The same parse puts skip=1e3 at offset 1 rather than 1000.

A repeated parameter is reduced to a guess. Express 5's simple query parser is the default this app uses (app.set('query parser') is never called), so ?limit=100&limit=200 arrives as an array, Number.parseInt(["100","200"], 10) coerces it to "100,200", and the server takes 100. ?limit=200&limit=100 gives 200. Neither is an error today.

Clamped limit is invisible. A clamped response carries Allow, Content-Type, Content-Length, an ETag, the JSON-LD context Link, and the CORS headers. No applied limit or skip, no maximum, no total. A client asking for 1000 and receiving 500 has nothing on the wire that distinguishes truncation from a genuine final page. This is how the problem was found.

Evidence

Verified 2026-09-02 and re-run 2026-09-03, read-only, on localhost:3001, devstore.rerum.io, and store.rerum.io. Full tables in the detailed report on #299.

Clamping is identical on all three deployments and both endpoints — limit=1000 and limit=5000 both return 500.

Querying {"__rerum.APIversion":{"$exists":true}} with limit=2, comparing the first returned id:

Consecutive pages at skip=100000 and skip=100100 with limit=100 return byte-identical id lists.

/search applies the same clamp but slices from its merged in-memory set, so the repeated page only appears for a term with more than 100000 merged matches. No term tried reaches that on the dev collection, so /search terminates today by accident of data, not by design. The mechanism is identical.

Affected lines

Proposed change

Validate the raw parameter before parsing, in getPagination(), so all six endpoints change together.

Reject with 400

• skip greater than the maximum. This is the change that converts the infinite loop into an immediate, legible failure:

  {
    "message": "The skip value 150000 exceeds the maximum of 100000. Follow the Link rel=\"next\" header or use a cursor to page deeper.",
    "status": 400
  }

• Any value that is not a decimal integer string: abc, 10abc, 1e3, 0x10, 250.7, 2.9, the empty string.

• limit of 0 or negative, and negative skip.

• A repeated parameter, which arrives as an array rather than a string.

Validating the raw value as a decimal integer string, and rejecting anything that is not a string, covers all of these in one place.

?limit[a]=5 is the one case that cannot be caught. Under the simple parser the server never receives a limit key at all, so it is indistinguishable from a request that omitted the parameter. Worth a line in the docs, not a code change.

Clamp and report

An over-maximum limit keeps returning the maximum, and says so. Report the applied values and the maximums on every paged response, not only on clamped ones, so a client can configure itself from any single response:

RERUM-Limit: 500
RERUM-Skip: 0
RERUM-Limit-Max: 500
RERUM-Skip-Max: 100000

Header naming is open. The codebase already sets unprefixed custom headers (Current-Overwritten-Version, controllers/gog.js:388), and RFC 6648 deprecates the X- prefix, so a RERUM- namespace is the suggestion rather than a convention already in place. Access-Control-Expose-Headers is already * (app.js:52), so browser clients can read whatever is chosen.

Notes

• Breaking. Requests that return 200 today will return 400. Land on dev first and give known client maintainers notice. The skip rejection is the part most likely to be hit by a real client, which is the point.
• The skip rejection is much easier to defend once /query paged responses carry no rel="next", so no client can tell a full page from the last page #302 and Add cursor-based continuation to /query so paging depth is unbounded and cost is flat #303 land, because clients then follow a link instead of computing offsets and depth stops being bounded at all. Consider sequencing the rejection after rel="next" ships so there is somewhere to send people.
• The 400 message names the configured maximum, so it has to be the real one. controllers/utils.js:12-13 reads RERUM_MAX_QUERY_LIMIT / RERUM_MAX_QUERY_SKIP while .env sets the unprefixed MAX_QUERY_LIMIT / MAX_QUERY_SKIP, and env-loader.js does no prefixing — so both caps currently fall back to the code defaults of 500 and 100000, and the skip cap of 10000 that .env asks for has never been in effect on either deployment. Reconcile the names as part of this work, and add a unit test that sets the key and asserts the resulting cap; the tests at __tests__/utils.test.js:329-354 pass under either name today, which is why the mismatch was never caught.
• The same PR should update public/API.html and the OpenAPI contract; see The pagination contract is undocumented, misdocumented, and absent from the OpenAPI contract #305 for the full sweep.

Acceptance criteria

• skip above the maximum returns 400 naming the maximum, rather than a repeated page
• limit=abc, limit=-5, limit=0, limit=1e3, limit=250.7, and the empty string return 400
• skip=abc, skip=-5, skip=1e3, and skip=2.9 return 400
• A repeated limit or skip parameter returns 400 rather than silently taking the first value
• limit above the maximum still returns the maximum, and the applied value and maximum appear in response headers
• The applied limit and skip are reported on every paged response, clamped or not
• /query, HEAD /query, /search, /search/phrase, and both /gog/*InManuscript endpoints behave identically for every case above
• Regression tests cover each rejected form and both clamp boundaries