From 71930257e538c638fdc22a8581397f8343cae9d8 Mon Sep 17 00:00:00 2001 From: Patrick Cuba Date: Thu, 2 Apr 2026 16:09:07 -0500 Subject: [PATCH 1/2] Add pagination parsing and tests, fix route checks Implement getPagination in rest.js to parse, validate, and cap query limit/skip values (defaults, non-negative integer enforcement, env-configured maximums, throws 400 on malformed input). Export getPagination and add unit tests (__tests__/rest.test.js) covering defaults, parsing, error cases, and env caps. Update routes: use rest.getPagination in routes/query.js to derive limit/skip from req.query and use optional chaining for id checks in routes/overwrite.js and routes/update.js. --- __tests__/rest.test.js | 39 +++++++++++++++++++++++++++++++++++++ package-lock.json | 28 ++------------------------- rest.js | 44 +++++++++++++++++++++++++++++++++++++++++- routes/overwrite.js | 2 +- routes/query.js | 16 ++++++--------- routes/update.js | 2 +- 6 files changed, 92 insertions(+), 39 deletions(-) create mode 100644 __tests__/rest.test.js diff --git a/__tests__/rest.test.js b/__tests__/rest.test.js new file mode 100644 index 0000000..ebe14aa --- /dev/null +++ b/__tests__/rest.test.js @@ -0,0 +1,39 @@ +import rest from "../rest.js" + +describe("rest.getPagination", () => { + const originalMaxLimit = process.env.RERUM_MAX_QUERY_LIMIT + const originalMaxSkip = process.env.RERUM_MAX_QUERY_SKIP + + afterEach(() => { + process.env.RERUM_MAX_QUERY_LIMIT = originalMaxLimit + process.env.RERUM_MAX_QUERY_SKIP = originalMaxSkip + }) + + it("returns defaults when values are omitted", () => { + const pagination = rest.getPagination({}, 10) + expect(pagination).toEqual({ limit: 10, skip: 0 }) + }) + + it("accepts explicit non-negative integer values", () => { + const pagination = rest.getPagination({ limit: "25", skip: "5" }, 10) + expect(pagination).toEqual({ limit: 25, skip: 5 }) + }) + + it("rejects malformed values with a 400 status", () => { + expect(() => rest.getPagination({ limit: "12abc", skip: "0" }, 10)).toThrow("`limit` and `skip` values must be non-negative integers or omitted.") + expect(() => rest.getPagination({ limit: "1", skip: "-2" }, 10)).toThrow("`limit` and `skip` values must be non-negative integers or omitted.") + expect(() => rest.getPagination({ limit: "1.5", skip: "0" }, 10)).toThrow("`limit` and `skip` values must be non-negative integers or omitted.") + try { + rest.getPagination({ limit: "hello", skip: "0" }, 10) + } catch (err) { + expect(err.status).toBe(400) + } + }) + + it("caps values to configured maximums", () => { + process.env.RERUM_MAX_QUERY_LIMIT = "50" + process.env.RERUM_MAX_QUERY_SKIP = "100" + const pagination = rest.getPagination({ limit: "1000", skip: "5000" }, 10) + expect(pagination).toEqual({ limit: 50, skip: 100 }) + }) +}) diff --git a/package-lock.json b/package-lock.json index 39ea4b6..938dbde 100644 --- a/package-lock.json +++ b/package-lock.json @@ -22,8 +22,8 @@ "supertest": "^7.2.2" }, "engines": { - "node": ">=24.12.0", - "npm": ">=11.7.0" + "node": ">=24.14.0", + "npm": ">=11.0.0" } }, "node_modules/@babel/code-frame": { @@ -1308,9 +1308,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1325,9 +1322,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -1342,9 +1336,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1359,9 +1350,6 @@ "riscv64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1376,9 +1364,6 @@ "riscv64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -1393,9 +1378,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1410,9 +1392,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1427,9 +1406,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ diff --git a/rest.js b/rest.js index e47a28c..c96a1cb 100644 --- a/rest.js +++ b/rest.js @@ -30,6 +30,45 @@ const hasMultipleContentTypes = (contentType) => { }) } +const DEFAULT_MAX_QUERY_LIMIT = 500 +const DEFAULT_MAX_QUERY_SKIP = 100000 + +/** + * Parse and bound query pagination values. + * + * @param {Object} [query={}] - Request query object + * @param {number} [defaultLimit=10] - Default limit when omitted + * @returns {{limit:number, skip:number}} + * @throws {Error} If limit or skip are not non-negative integers + */ +const getPagination = (query = {}, defaultLimit = 10) => { + const maxLimit = Number.parseInt(process.env.RERUM_MAX_QUERY_LIMIT ?? `${DEFAULT_MAX_QUERY_LIMIT}`, 10) + const maxSkip = Number.parseInt(process.env.RERUM_MAX_QUERY_SKIP ?? `${DEFAULT_MAX_QUERY_SKIP}`, 10) + const safeMaxLimit = Number.isFinite(maxLimit) && maxLimit > 0 ? maxLimit : DEFAULT_MAX_QUERY_LIMIT + const safeMaxSkip = Number.isFinite(maxSkip) && maxSkip >= 0 ? maxSkip : DEFAULT_MAX_QUERY_SKIP + + const parseNonNegativeInteger = (value, fallback) => { + if (value === undefined || value === null || value === "") return fallback + const normalized = `${value}`.trim() + if (!/^\d+$/.test(normalized)) return null + return Number.parseInt(normalized, 10) + } + + const limit = parseNonNegativeInteger(query.limit, defaultLimit) + const skip = parseNonNegativeInteger(query.skip, 0) + + if (limit === null || skip === null) { + const err = new Error("`limit` and `skip` values must be non-negative integers or omitted.") + err.status = 400 + throw err + } + + return { + limit: Math.min(limit, safeMaxLimit), + skip: Math.min(skip, safeMaxSkip) + } +} + /** * Middleware to verify Content-Type headers for endpoints receiving JSON bodies. * Responds with a 415 Invalid Media Type for Content-Type headers that are not for JSON bodies. @@ -57,4 +96,7 @@ const verifyJsonContentType = function (req, res, next) { return next(err) } -export default { verifyJsonContentType } +export default { + getPagination, + verifyJsonContentType +} diff --git a/routes/overwrite.js b/routes/overwrite.js index b09a42a..dc6d68d 100644 --- a/routes/overwrite.js +++ b/routes/overwrite.js @@ -11,7 +11,7 @@ router.put('/', rest.verifyJsonContentType, checkAccessToken, async (req, res, n const overwriteBody = req.body // check for @id; any value is valid - if (!(overwriteBody['@id'] ?? overwriteBody.id)) { + if (!(overwriteBody?.['@id'] ?? overwriteBody?.id)) { const err = new Error("No record id to overwrite! (https://store.rerum.io/API.html#overwrite)") err.status = 400 throw err diff --git a/routes/query.js b/routes/query.js index fcc45f4..e799971 100644 --- a/routes/query.js +++ b/routes/query.js @@ -5,9 +5,8 @@ const router = express.Router() /* POST a query to the thing. */ router.post('/', rest.verifyJsonContentType, async (req, res, next) => { - const lim = req.query.limit ?? 10 - const skip = req.query.skip ?? 0 try { + const { limit, skip } = rest.getPagination(req.query, 10) // check body for JSON const queryBody = JSON.stringify(req.body) // If there is an empty query with [] or {}, we consider that a query for all data, @@ -17,14 +16,6 @@ router.post('/', rest.verifyJsonContentType, async (req, res, next) => { err.status = 400 throw err } - // check limit and skip for INT - if (isNaN(parseInt(lim) + parseInt(skip)) - || (lim < 0) - || (skip < 0)) { - const err = new Error("`limit` and `skip` values must be positive integers or omitted.") - err.status = 400 - throw err - } const queryOptions = { method: 'POST', body: queryBody, @@ -35,8 +26,13 @@ router.post('/', rest.verifyJsonContentType, async (req, res, next) => { 'Content-Type' : "application/json;charset=utf-8" } } +<<<<<<< Updated upstream const queryURL = `${process.env.RERUM_API_ADDR}query?limit=${lim}&skip=${skip}` const rerumResponse = await fetch(queryURL, queryOptions) +======= + const queryURL = `${process.env.RERUM_API_ADDR}query?limit=${limit}&skip=${skip}` + const rerumResponse = await fetchRerum(queryURL, queryOptions) +>>>>>>> Stashed changes .then(async (resp) => { if (resp.ok) return resp.json() // The response from RERUM indicates a failure, likely with a specific code and textual body diff --git a/routes/update.js b/routes/update.js index a025875..b4d674a 100644 --- a/routes/update.js +++ b/routes/update.js @@ -9,7 +9,7 @@ router.put('/', rest.verifyJsonContentType, checkAccessToken, async (req, res, n try { // check for @id; any value is valid - if (!(req.body['@id'] ?? req.body.id)) { + if (!(req.body?.['@id'] ?? req.body?.id)) { const err = new Error("No record id to update! (https://store.rerum.io/API.html#update)") err.status = 400 throw err From 5ebcfeeb0150ce6f4b1846b2a4a1a889f2b76cbe Mon Sep 17 00:00:00 2001 From: Patrick Cuba Date: Thu, 2 Apr 2026 16:20:27 -0500 Subject: [PATCH 2/2] Add pagination parsing and tests, fix route checks Implement getPagination in rest.js to parse, validate, and cap query limit/skip values (defaults, non-negative integer enforcement, env-configured maximums, throws 400 on malformed input). Export getPagination and add unit tests (__tests__/rest.test.js) covering defaults, parsing, error cases, and env caps. Update routes: use rest.getPagination in routes/query.js to derive limit/skip from req.query and use optional chaining for id checks in routes/overwrite.js and routes/update.js. --- routes/query.js | 12 +----------- routes/update.js | 13 +++---------- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/routes/query.js b/routes/query.js index e799971..8bf3034 100644 --- a/routes/query.js +++ b/routes/query.js @@ -1,5 +1,6 @@ import express from "express" import rest from "../rest.js" +import { fetchRerum } from "../rerum.js" const router = express.Router() @@ -26,13 +27,8 @@ router.post('/', rest.verifyJsonContentType, async (req, res, next) => { 'Content-Type' : "application/json;charset=utf-8" } } -<<<<<<< Updated upstream - const queryURL = `${process.env.RERUM_API_ADDR}query?limit=${lim}&skip=${skip}` - const rerumResponse = await fetch(queryURL, queryOptions) -======= const queryURL = `${process.env.RERUM_API_ADDR}query?limit=${limit}&skip=${skip}` const rerumResponse = await fetchRerum(queryURL, queryOptions) ->>>>>>> Stashed changes .then(async (resp) => { if (resp.ok) return resp.json() // The response from RERUM indicates a failure, likely with a specific code and textual body @@ -46,12 +42,6 @@ router.post('/', rest.verifyJsonContentType, async (req, res, next) => { err.status = 502 throw err }) - .catch(err => { - if (err.status === 502) throw err - const genericRerumNetworkError = new Error(`500: ${queryURL} - A RERUM error occurred`) - genericRerumNetworkError.status = 502 - throw genericRerumNetworkError - }) res.status(200).json(rerumResponse) } catch (err) { diff --git a/routes/update.js b/routes/update.js index b4d674a..01ef290 100644 --- a/routes/update.js +++ b/routes/update.js @@ -1,6 +1,7 @@ import express from "express" import checkAccessToken from "../tokens.js" import rest from "../rest.js" +import { createRerumNetworkError, fetchRerum } from "../rerum.js" const router = express.Router() @@ -27,7 +28,7 @@ router.put('/', rest.verifyJsonContentType, checkAccessToken, async (req, res, n } } const updateURL = `${process.env.RERUM_API_ADDR}update` - const rerumResponse = await fetch(updateURL, updateOptions) + const rerumResponse = await fetchRerum(updateURL, updateOptions) .then(async (resp) => { if (resp.ok) return resp.json() // The response from RERUM indicates a failure, likely with a specific code and textual body @@ -41,17 +42,9 @@ router.put('/', rest.verifyJsonContentType, checkAccessToken, async (req, res, n err.status = 502 throw err }) - .catch(err => { - if (err.status === 502) throw err - const genericRerumNetworkError = new Error(`500: ${updateURL} - A RERUM error occurred`) - genericRerumNetworkError.status = 502 - throw genericRerumNetworkError - }) if (!(rerumResponse.id || rerumResponse["@id"])) { // A 200 with garbled data, call it a fail - const genericRerumNetworkError = new Error(`500: ${updateURL} - A RERUM error occurred`) - genericRerumNetworkError.status = 502 - throw genericRerumNetworkError + throw createRerumNetworkError(updateURL) } res.setHeader("Location", rerumResponse["@id"] ?? rerumResponse.id) res.status(200).json(rerumResponse)