From 7683d8186d063b390ad39b2fd8d0d08d0c27265c Mon Sep 17 00:00:00 2001 From: derrickmehaffy Date: Mon, 12 Oct 2026 18:40:34 -0700 Subject: [PATCH] fix(recv): do not cache Buffer bodies, which replay as JSON isCacheable accepted any non-empty, non-stream body, and the store serialises every entry with JSON.stringify({ data }). A Buffer stringifies to { type: "Buffer", data: [...] } and nothing turns it back, and the handler's Content-Type is not stored either. So the first caller got the CSV or file the handler sent, and every caller after them got a JSON array of byte values until maxAge. Refuse a Buffer body in isCacheable, the way a stream already is: the response goes out untouched and nothing is stored. Storing binary faithfully would mean a new entry format and storing headers, which is a much larger change than the bug. Only Buffer is refused because it is the only body Koa sends as raw bytes; it JSON-encodes any other typed array itself, and that round-trips through the store unchanged, so what is cached for every other body stays exactly as it was. --- docs/guide/caching/index.md | 1 + docs/guide/reference/config.md | 2 + .../src/utils/middlewares/isCacheable.ts | 10 ++++ shared/config/cache-strategy.js | 1 + .../src/api/category/controllers/category.js | 6 ++ .../api/category/routes/custom-category.js | 5 ++ shared/src/bootstrap.js | 1 + shared/tests/buffer-body.test.js | 58 +++++++++++++++++++ 8 files changed, 84 insertions(+) create mode 100644 shared/tests/buffer-body.test.js diff --git a/docs/guide/caching/index.md b/docs/guide/caching/index.md index 23f4d0ab..5dd7ffe8 100644 --- a/docs/guide/caching/index.md +++ b/docs/guide/caching/index.md @@ -203,6 +203,7 @@ holds for every response. | The body is empty | There is nothing to replay. | | The status is not `2xx` | Errors and redirects are not the answer to the question that was asked. | | The body is a stream | A stream can be consumed once. A stored copy cannot be replayed, and the entry would poison every later request for that key. | +| The body is a `Buffer` | Entries are stored as JSON, which cannot carry raw bytes or the `Content-Type`. A replayed file or CSV would arrive as a JSON array of byte values. | | The response sets `Set-Cookie` | A cookie is issued to one caller. Replaying it hands that caller's session, CSRF token or consent state to everybody sharing the key. | | `Cache-Control` says `no-store` or `private` | An explicit instruction from the handler not to store the response. | diff --git a/docs/guide/reference/config.md b/docs/guide/reference/config.md index 931c4e65..a2826857 100644 --- a/docs/guide/reference/config.md +++ b/docs/guide/reference/config.md @@ -319,6 +319,8 @@ Independently of configuration, a response is not stored when: - the body is empty, or the status is not `2xx`; - the body is a stream — it can only be consumed once, so a stored copy could not be replayed; +- the body is a `Buffer` — entries are stored as JSON, which cannot carry + raw bytes or their `Content-Type`; - the response sets a `Set-Cookie` — replaying it would hand one caller's session to everybody sharing the key; - the response says `Cache-Control: no-store` or `private`. diff --git a/packages/plugin-rest-cache/server/src/utils/middlewares/isCacheable.ts b/packages/plugin-rest-cache/server/src/utils/middlewares/isCacheable.ts index 5878d7e2..9f0c61e3 100644 --- a/packages/plugin-rest-cache/server/src/utils/middlewares/isCacheable.ts +++ b/packages/plugin-rest-cache/server/src/utils/middlewares/isCacheable.ts @@ -36,6 +36,16 @@ export const isCacheable = function (ctx: Context): CacheabilityVerdict { return { cacheable: false, reason: 'streamed body' }; } + // Entries are stored as JSON.stringify({ data }), and a Buffer stringifies + // to { type: "Buffer", data: [...] }, which JSON.parse never turns back into + // a Buffer. Nor is the Content-Type stored. So every HIT served a JSON array + // of byte values in place of the file or CSV the handler sent. Koa sends only + // a Buffer as raw bytes - any other typed array it serialises as JSON itself, + // which round-trips - so a Buffer is the one binary body refused here. + if (Buffer.isBuffer(ctx.body)) { + return { cacheable: false, reason: 'binary (Buffer) body' }; + } + // Set-Cookie is issued to one caller. Replaying it hands that caller's // session, CSRF token or consent state to everybody else who shares the key. const setCookie = ctx.response.get('Set-Cookie'); diff --git a/shared/config/cache-strategy.js b/shared/config/cache-strategy.js index 40baf6b2..7820d506 100644 --- a/shared/config/cache-strategy.js +++ b/shared/config/cache-strategy.js @@ -70,6 +70,7 @@ module.exports = ({ env }) => ({ { path: "/api/categories/probe/raw", method: "GET" }, { path: "/api/categories/probe/stream", method: "GET" }, { path: "/api/categories/probe/with-cookie", method: "GET" }, + { path: "/api/categories/probe/buffer", method: "GET" }, // No maxAge, hitpass or keys of its own: all three must come from // this content type, not from the strategy. See // https://github.com/strapi-community/plugin-rest-cache/issues/230 diff --git a/shared/src/api/category/controllers/category.js b/shared/src/api/category/controllers/category.js index efe42459..71b0bebf 100644 --- a/shared/src/api/category/controllers/category.js +++ b/shared/src/api/category/controllers/category.js @@ -27,6 +27,12 @@ module.exports = createCoreController( ctx.body = Readable.from([JSON.stringify({ streamed: true })]); }, + /** A Buffer body with its own content type, e.g. a proxied file. */ + async buffer(ctx) { + ctx.type = "text/csv"; + ctx.body = Buffer.from("a,b\n1,2\n"); + }, + /** A response carrying a Set-Cookie, which must never be shared. */ async withCookie(ctx) { ctx.cookies.set("session", `s-${Date.now()}`, { httpOnly: true }); diff --git a/shared/src/api/category/routes/custom-category.js b/shared/src/api/category/routes/custom-category.js index c65e15d3..8243467a 100644 --- a/shared/src/api/category/routes/custom-category.js +++ b/shared/src/api/category/routes/custom-category.js @@ -19,6 +19,11 @@ module.exports = { path: "/categories/probe/stream", handler: "category.stream", }, + { + method: "GET", + path: "/categories/probe/buffer", + handler: "category.buffer", + }, { method: "GET", path: "/categories/probe/with-cookie", diff --git a/shared/src/bootstrap.js b/shared/src/bootstrap.js index 919f087d..43983f6a 100644 --- a/shared/src/bootstrap.js +++ b/shared/src/bootstrap.js @@ -192,6 +192,7 @@ async function importSeedData() { "findOne", "raw", "stream", + "buffer", "withCookie", "withCacheControl", "inherits", diff --git a/shared/tests/buffer-body.test.js b/shared/tests/buffer-body.test.js new file mode 100644 index 00000000..20e94634 --- /dev/null +++ b/shared/tests/buffer-body.test.js @@ -0,0 +1,58 @@ +"use strict"; + +/** + * A Buffer body must reach every caller as the bytes the handler sent. + * + * cacheStore.set serialises every body with JSON.stringify({ data }). A Buffer + * stringifies to { type: "Buffer", data: [...] }, and JSON.parse never turns + * that back into a Buffer, so every HIT used to serve that JSON object instead + * of the original bytes - and without the Content-Type the handler set, which + * is not stored either. A CSV export or a proxied file became a JSON array of + * byte values for everyone after the first caller. + * + * Such a body is now refused by isCacheable, like a stream: returned as normal, + * never stored. + */ + +const { setup, teardown, agent } = require("./helpers/strapi"); + +jest.setTimeout(60000); + +process.env.STRAPI_DISABLE_UPDATE_NOTIFICATION = true; +process.env.STRAPI_HIDE_STARTUP_MESSAGE = true; +process.env.STRAPI_TELEMETRY_DISABLED = true; + +const store = () => strapi.plugin("rest-cache").service("cacheStore"); + +describe("Buffer bodies", () => { + beforeAll(async () => { + process.env.KEYS_PREFIX = undefined; + await setup(); + }); + afterAll(async () => await teardown()); + + beforeEach(() => store().reset()); + + it("serves the same bytes and type to a second caller", async () => { + const first = await agent().get("/api/categories/probe/buffer").buffer(true); + const second = await agent().get("/api/categories/probe/buffer").buffer(true); + + expect(first.status).toBe(200); + expect(first.get("x-cache")).toBe("MISS"); + expect(first.text).toBe("a,b\n1,2\n"); + expect(first.get("content-type")).toMatch(/^text\/csv/); + + expect(second.status).toBe(200); + expect(second.text).toBe(first.text); + expect(second.get("content-type")).toBe(first.get("content-type")); + // Refused rather than stored, so the second caller reaches the origin. + expect(second.get("x-cache")).toBe("MISS"); + }); + + it("does not store anything for it", async () => { + await agent().get("/api/categories/probe/buffer"); + + const keys = await store().keys(); + expect(keys.filter((k) => k.includes("/categories/probe/buffer"))).toHaveLength(0); + }); +});