diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index e7856a9..e6a8005 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -5,9 +5,9 @@ jobs: testing: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v1 + - uses: actions/checkout@v4 - name: Setup Environment (Using NodeJS 22.x) - uses: actions/setup-node@v1 + uses: actions/setup-node@v4 with: node-version: 22.x @@ -15,7 +15,7 @@ jobs: run: npm install - name: Linting - run: npm run format + run: npx prettier --check *.js - name: Run tests run: npm run test diff --git a/.gitignore b/.gitignore index 01b8cda..19fd222 100644 --- a/.gitignore +++ b/.gitignore @@ -178,3 +178,4 @@ dist *-lock.json bun.lockb +sec-findings.md diff --git a/README.md b/README.md index 3ff1789..e5d5452 100644 --- a/README.md +++ b/README.md @@ -5,9 +5,12 @@ This middleware enables your API to handle requests idempotently, ensuring that ## Features - **Idempotent Request Handling**: Ensures that duplicate requests with the same idempotency key are processed only once, preventing unintended side effects. +- **Original Response Replay**: On a cache hit, the middleware replays the original HTTP status code, headers, and body instead of returning a generic empty response. +- **Concurrent Request Deduplication**: Uses an in-flight lock so that simultaneous requests with the same idempotency key execute the handler only once. +- **Cache Key Scoping**: Cache keys are derived from the HTTP method, full request URL (including query string), and idempotency key to prevent cross-route and cross-method collisions. - **Customizable Cache Integration**: Supports any cache library that implements `get` and `set` methods, allowing flexibility in your caching strategy. - **Configurable Idempotency Key**: Lets you define the key used to identify requests. By default, it uses the `x-request-id` header. -- **Adjustable TTL (Time-to-Live)**: Provides the ability to configure the expiration time for cache entries, balancing performance and resource usage. +- **Adjustable TTL (Time-to-Live)**: Provides the ability to configure the expiration time for cache entries, balancing performance and resource usage (max 24 hours). - **HTTP Method Support**: Compatible with the following HTTP methods: `POST`, `PUT`, `PATCH`, and `DELETE`. ## Installation @@ -63,24 +66,51 @@ Calling the API ```bash curl -X POST http://localhost:3000/create -H "x-request-id: 123" # 200 -> Resource created! -curl -X POST http://localhost:3000/create -H "x-request-id: 123" # 204 +curl -X POST http://localhost:3000/create -H "x-request-id: 123" # 200 -> Resource created! (replayed from cache) # after 5 seconds curl -X POST http://localhost:3000/create -H "x-request-id: 123" # 200 -> Resource created! ``` +## Options + +| Option | Type | Default | Description | +| ------------------------- | -------------------------------------- | ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `cache` | `Cache` | required | A cache instance with `.get(key)` and `.set(key, value, {ttl})` methods. | +| `ttl` | `number` | required | Cache TTL in milliseconds. Must be between `1` and `86,400,000` (24 hours). | +| `idempotencyKeyExtractor` | `(req) => string \| undefined \| null` | `req.headers['x-request-id']` | Extracts the idempotency key from the request. The returned key must match `^[a-zA-Z0-9_.~-]{1,128}$`. Duplicate headers are exposed as arrays by Node.js; the default extractor does not normalize them, so idempotency is skipped for such requests. | +| `keyPrefix` | `string` | `'idemp-key-'` | Prefix prepended to every cache key. | +| `maxResponseSize` | `number` | `1,048,576` (1 MB) | Maximum response body size (in bytes) that will be cached. Larger responses are not cached. | +| `logger` | `Logger` | `console` | Logger used for error reporting. Must expose an `.error(...args)` method. | + +### Behavior notes + +- Only successful responses with a `2xx` status code are cached. +- Hop-by-hop and connection-level headers such as `Connection`, `Keep-Alive`, `Transfer-Encoding`, `Content-Length`, and `Date` are stripped before replay and are not restored from the cache. +- Responses larger than `maxResponseSize` are still served to the client; only the cache write is skipped. +- Previous versions stored a plain string (`"1"`) in the cache. Those entries are ignored after upgrading, so only new responses will be replayed. + ## Customizing idempotency key By default, the middleware uses the `x-request-id` header to identify the request. You can customize the key that will be used to identify the request by passing a custom `idempotencyKeyExtractor` function to the middleware. -> In production environments, it is recommended to use a combination of the `x-request-id` header and other unique identifiers such as `service-name` and `user-id` to ensure the key's uniqueness and prevent collisions. +> In production environments, it is **strongly recommended** to combine the `x-request-id` header with user/tenant identifiers (e.g., a hashed user ID or session token) to ensure the key's uniqueness and prevent cross-user collisions. ```javascript +function extractIdempotencyKey(req: Request) { + const header = req.headers['x-custom-req-id'] + const value = Array.isArray(header) ? header[0] : header + if (!value || !/^[a-zA-Z0-9_.~-]{1,128}$/.test(value)) { + return undefined + } + // Scope the key with a service and user identifier to prevent cross-user collisions. + const userId = req.user?.id ?? 'anonymous' + return `${SERVICE_NAME}-${userId}-${value}` +} + app.use( idempotencyMiddleware({ ttl: 5000, - idempotencyKeyExtractor: (req: Request) => { - return `${SERVICE_NAME}-${req.headers['x-custom-req-id']}` - }, + idempotencyKeyExtractor: extractIdempotencyKey, //..., }), ) @@ -88,7 +118,7 @@ app.use( ### Security Considerations -The middleware is designed to operate in a trusted environment. If you plan to deploy it in an untrusted or partially trusted environment, take the following risks and mitigations into account: +The middleware is designed to operate safely in untrusted or partially trusted environments when configured correctly. Keep the following risks and mitigations in mind: #### 1. **Cache Flooding** @@ -98,6 +128,7 @@ An attacker could overwhelm the cache by sending a high volume of requests with - Implement rate limiting and throttling mechanisms at the middleware or API gateway level. - Set a maximum capacity for the idempotency cache, with a defined eviction policy (e.g., Least Recently Used (LRU) strategy). +- Use the `maxResponseSize` option to avoid caching very large responses. - Monitor and log unusual traffic patterns to detect and respond to potential attacks promptly. #### 2. **Identity Spoofing** @@ -109,6 +140,14 @@ An attacker could forge the `x-request-id` header to impersonate another user's - Use a secure idempotency key that combines the `x-request-id` header with user-specific information, such as a hashed user identifier or session token. - Encrypt or digitally sign the `x-request-id` value to ensure its authenticity and prevent tampering. +#### 3. **Concurrent Duplicate Processing** + +Without locking, two simultaneous requests with the same idempotency key could both execute the underlying handler. + +**Mitigation:** + +- This middleware now maintains an in-flight lock per idempotency key so that duplicate concurrent requests wait for the first request to finish and then replay its cached response. + #### General Recommendations - Regularly audit the middleware's security practices and ensure compliance with your organization's security standards. diff --git a/demos/express.js b/demos/express.js index ee70325..c709d50 100644 --- a/demos/express.js +++ b/demos/express.js @@ -6,6 +6,12 @@ const {CacheableMemory} = require('cacheable') const SERVICE_NAME = 'express-demo' +function getCurrentUserId(req) { + // Replace this with the real authenticated user identifier. + // In Express you typically read it from req.user after authentication. + return req.user?.id ?? 'anonymous' +} + const cache = createCache({ stores: [ new Keyv({ @@ -15,13 +21,22 @@ const cache = createCache({ ], }) +function extractIdempotencyKey(req) { + const header = req.headers['x-custom-req-id'] + const value = Array.isArray(header) ? header[0] : header + if (!value || !/^[a-zA-Z0-9_.~-]{1,128}$/.test(value)) { + return undefined + } + // Scope the key with a service and user identifier to prevent cross-user collisions. + return `${SERVICE_NAME}-${getCurrentUserId(req)}-${value}` +} + const app = express() app.use( idempotencyMiddleware({ ttl: 5000, - idempotencyKeyExtractor: (req) => - `${SERVICE_NAME}-${req.headers['x-custom-req-id']}`, + idempotencyKeyExtractor: extractIdempotencyKey, cache: { get: async (key) => { return cache.get(key) diff --git a/demos/express.ts b/demos/express.ts index 3d9cef4..71a87df 100644 --- a/demos/express.ts +++ b/demos/express.ts @@ -6,6 +6,12 @@ import {CacheableMemory} from 'cacheable' const SERVICE_NAME = 'express-demo' +function getCurrentUserId(req: Request): string { + // Replace this with the real authenticated user identifier. + // In Express you typically read it from req.user after authentication. + return (req as Request & {user?: {id: string}}).user?.id ?? 'anonymous' +} + const cache = createCache({ stores: [ new Keyv({ @@ -15,13 +21,22 @@ const cache = createCache({ ], }) +function extractIdempotencyKey(req: Request): string | undefined { + const header = req.headers['x-custom-req-id'] + const value = Array.isArray(header) ? header[0] : header + if (!value || !/^[a-zA-Z0-9_.~-]{1,128}$/.test(value)) { + return undefined + } + // Scope the key with a service and user identifier to prevent cross-user collisions. + return `${SERVICE_NAME}-${getCurrentUserId(req)}-${value}` +} + const app = express() app.use( idempotencyMiddleware({ ttl: 5000, - idempotencyKeyExtractor: (req) => - `${SERVICE_NAME}-${req.headers['x-custom-req-id']}`, + idempotencyKeyExtractor: extractIdempotencyKey, cache: { get: async (key: string) => { return cache.get(key) diff --git a/index.d.ts b/index.d.ts index 3c27dc0..0e9ca20 100644 --- a/index.d.ts +++ b/index.d.ts @@ -59,6 +59,7 @@ export interface IdempotencyMiddlewareOptions { /** * Default time-to-live for cached responses, in milliseconds. + * Must be between 1 and 86,400,000 (24 hours). */ ttl: number @@ -66,10 +67,14 @@ export interface IdempotencyMiddlewareOptions { * A function to extract the idempotency key from the HTTP request. * Defaults to extracting the `x-request-id` header. * + * The returned key must be a non-empty string of at most 128 characters + * matching `^[a-zA-Z0-9_.~-]+$`. In multi-user environments the key should + * be scoped with a user/tenant identifier to prevent cross-user collisions. + * * @param req - The incoming HTTP request. - * @returns The extracted idempotency key as a string. + * @returns The extracted idempotency key, or `undefined`/`null` to disable idempotency. */ - idempotencyKeyExtractor?: (req: IncomingMessage) => string + idempotencyKeyExtractor?: (req: IncomingMessage) => string | undefined | null /** * An optional logger for error reporting. @@ -80,9 +85,22 @@ export interface IdempotencyMiddlewareOptions { /** * A prefix to prepend to cache keys to avoid collisions with other cached data. - * Defaults to `'idempotent-req-'`. + * Defaults to `'idemp-key-'`. */ keyPrefix?: string + + /** + * Maximum response body size (in bytes) that will be cached. + * Responses larger than this value are not cached, preventing cache memory exhaustion. + * Defaults to 1 MB (1,048,576 bytes). + */ + maxResponseSize?: number + + /** + * Maximum time (in milliseconds) to wait for a cache read before giving up and + * calling the next handler. Defaults to 5,000 ms. + */ + cacheTimeout?: number } /** @@ -90,15 +108,23 @@ export interface IdempotencyMiddlewareOptions { * * The middleware ensures idempotent handling of requests by: * - Checking if a response for a unique request key (idempotency key) is cached. - * - Returning a cached response if available, skipping reprocessing. + * - Returning the cached response if available, skipping reprocessing. * - Caching successful responses (status codes 2xx) for future requests. * + * The cache key is scoped by HTTP method, request URL, and the extracted idempotency + * key, and the middleware uses an in-flight lock to prevent concurrent duplicate + * processing of the same key. + * * @param options - Configuration options for the middleware. * @returns A middleware function compatible with Connect-like frameworks. */ export function idempotencyMiddleware( options: IdempotencyMiddlewareOptions, -): (req: IncomingMessage, res: ServerResponse, next: () => void) => void +): ( + req: IncomingMessage, + res: ServerResponse, + next: (err?: any) => void, +) => void /** * Generates a SHA-256 hash of the input string. diff --git a/index.js b/index.js index 922d965..e4a5a63 100644 --- a/index.js +++ b/index.js @@ -3,6 +3,26 @@ import crypto from 'crypto' import onEnd from 'on-http-end' +const DEFAULT_KEY_PREFIX = 'idemp-key-' +const MAX_KEY_LENGTH = 128 +const SAFE_KEY_PATTERN = /^[a-zA-Z0-9_.~-]+$/ +const MAX_TTL_MS = 24 * 60 * 60 * 1000 // 24 hours +const DEFAULT_MAX_RESPONSE_SIZE = 1024 * 1024 // 1 MB +const DEFAULT_CACHE_TIMEOUT = 5000 // 5 seconds + +const HOP_BY_HOP_HEADERS = new Set([ + 'connection', + 'keep-alive', + 'proxy-authenticate', + 'proxy-authorization', + 'te', + 'trailer', + 'transfer-encoding', + 'upgrade', + 'content-length', + 'date', +]) + /** * Creates a middleware function that implements idempotency based on a request-specific key * (commonly referred to as an 'idempotency key' or 'request id'). @@ -13,19 +33,25 @@ import onEnd from 'on-http-end' * * @param {Object} options - Configuration options for the middleware. * @param {Object} options.cache - A cache instance that supports `.get(key)` and `.set(key, value, { ttl })` methods. - * @param {number} options.ttl - Time-to-live in milliseconds for cached responses. + * @param {number} options.ttl - Time-to-live in milliseconds for cached responses (1..86400000). * @param {string} [options.idempotencyKeyExtractor] - A function that extracts the idempotency key from the request object. * @param {Object} [options.logger=console] - A logger object with `.error()` and possibly other methods for logging. + * @param {string} [options.keyPrefix='idemp-key-'] - Prefix prepended to cache keys. + * @param {number} [options.maxResponseSize=1048576] - Maximum response body size (in bytes) that will be cached. + * @param {number} [options.cacheTimeout=5000] - Maximum time (in milliseconds) to wait for cache.get() before timing out. * * @returns {Function} Connect-style middleware function `(req, res, next)`. * - * @throws {Error} If `cache` or `ttl` is not provided. + * @throws {Error} If `cache` or `ttl` is not provided, or if options are invalid. */ export function idempotencyMiddleware({ cache, ttl, idempotencyKeyExtractor = (req) => req.headers['x-request-id'], logger = console, + keyPrefix = DEFAULT_KEY_PREFIX, + maxResponseSize = DEFAULT_MAX_RESPONSE_SIZE, + cacheTimeout = DEFAULT_CACHE_TIMEOUT, }) { // Validate the mandatory parameters if ( @@ -38,75 +64,277 @@ export function idempotencyMiddleware({ ) } - if (typeof ttl !== 'number' || ttl <= 0) { + if (typeof ttl !== 'number' || !Number.isFinite(ttl) || ttl <= 0) { throw new Error( 'IdempotencyMiddleware: A positive numeric ttl (in milliseconds) is required.', ) } - return function (req, res, next) { - if ( - req.method === 'POST' || - req.method === 'PUT' || - req.method === 'PATCH' || - req.method === 'DELETE' - ) { - let idempotencyKey = idempotencyKeyExtractor(req) - - // If no idempotency key is found, there's no special handling needed - if (typeof idempotencyKey !== 'string' || idempotencyKey.length === 0) { + if (ttl > MAX_TTL_MS) { + throw new Error( + `IdempotencyMiddleware: ttl must be between 1 and ${MAX_TTL_MS} milliseconds.`, + ) + } + + if (typeof keyPrefix !== 'string' || keyPrefix.length === 0) { + throw new Error( + 'IdempotencyMiddleware: keyPrefix must be a non-empty string.', + ) + } + + if ( + typeof maxResponseSize !== 'number' || + !Number.isFinite(maxResponseSize) || + maxResponseSize <= 0 + ) { + throw new Error( + 'IdempotencyMiddleware: maxResponseSize must be a positive number.', + ) + } + + if ( + typeof cacheTimeout !== 'number' || + !Number.isFinite(cacheTimeout) || + cacheTimeout <= 0 + ) { + throw new Error( + 'IdempotencyMiddleware: cacheTimeout must be a positive number.', + ) + } + + // In-flight request locks per cache key. This prevents two requests with the same + // idempotency key from both missing the cache and executing the handler. + const inFlight = new Map() + + return async function (req, res, next) { + try { + if ( + req.method !== 'POST' && + req.method !== 'PUT' && + req.method !== 'PATCH' && + req.method !== 'DELETE' + ) { return next() } - // Hash the idempotency key to ensure it's a valid cache key - idempotencyKey = 'idemp-key-' + hashSha256(idempotencyKey) - - // Attempt to retrieve a cached response - cache - .get(idempotencyKey) - .then(function (cachedResponse) { - if (cachedResponse) { - res.statusCode = 204 - res.setHeader('X-Idempotency-Status', 'hit') - res.setHeader('Content-Type', 'text/plain; charset=utf-8') - res.end('') // Ensuring protocol consistency - } else { - // No cached response found: set up a post-response hook - onEnd(res, function (payload) { - // Only cache the response if it's a success (2xx) - if ( - payload.status >= 200 && - payload.status < 300 && - typeof idempotencyKey === 'string' - ) { - // Store a simple flag or a derived response as needed. - // Here we store `true` to indicate a successful processed request. - // If you need to store the actual payload, store `payload.body` instead. - cache - .set(idempotencyKey, '1', {ttl: ttl}) - .catch(function (err) { - logger.error( - 'IdempotencyMiddleware - Cache WRITE Error:', - err, - ) - }) - } - }) + let idempotencyKey + try { + idempotencyKey = idempotencyKeyExtractor(req) + } catch (err) { + logger.error('IdempotencyMiddleware - Extractor Error:', err) + return next() + } + + if (!isValidIdempotencyKey(idempotencyKey)) { + return next() + } + + const cacheKey = buildCacheKey(keyPrefix, req, idempotencyKey) - // Proceed to the next handler in the chain - next() + // Wait for any in-flight request for the same key to finish, then acquire the + // lock before any async cache read so concurrent requests cannot race past + // this point and both execute the handler. + let existing = inFlight.get(cacheKey) + while (existing) { + await existing + existing = inFlight.get(cacheKey) + } + + const {promise: lock, release} = createLock() + inFlight.set(cacheKey, lock) + + const onClose = () => release() + res.once('close', onClose) + + try { + // Double-check the cache now that we hold the lock. + const cachedResponse = await withTimeout( + cache.get(cacheKey), + cacheTimeout, + ) + if (isValidCachedResponse(cachedResponse)) { + replayResponse(res, cachedResponse) + res.removeListener('close', onClose) + release() + inFlight.delete(cacheKey) + return + } + + // No cached response found: set up a post-response hook. + onEnd(res, function (payload) { + try { + // Only cache the response if it's a success (2xx) and not too large. + if ( + payload.status >= 200 && + payload.status < 300 && + typeof cacheKey === 'string' && + getBodyLength(payload.data) <= maxResponseSize + ) { + const responseToCache = { + version: 1, + status: payload.status, + headers: payload.headers, + body: serializeBody(payload.data), + cachedAt: Date.now(), + } + cache + .set(cacheKey, responseToCache, {ttl: ttl}) + .catch(function (err) { + logger.error( + 'IdempotencyMiddleware - Cache WRITE Error:', + err, + ) + }) + } + } finally { + res.removeListener('close', onClose) + release() + inFlight.delete(cacheKey) } }) - .catch(function (error) { - // If there's an error reading from the cache, log and proceed without caching - logger.error('IdempotencyMiddleware - Cache READ Error:', error) - next() - }) - } else { - // For non-idempotent methods, proceed without special handling - next() + + // Proceed to the next handler in the chain + next() + } catch (error) { + res.removeListener('close', onClose) + release() + inFlight.delete(cacheKey) + if ( + error.message === + 'IdempotencyMiddleware - Response headers already sent' + ) { + return next(error) + } + logger.error('IdempotencyMiddleware - Cache READ Error:', error) + return next() + } + } catch (error) { + logger.error('IdempotencyMiddleware - Unexpected Error:', error) + return next(error) + } + } +} + +function createLock() { + let release + let released = false + const promise = new Promise((resolve) => { + release = () => { + if (released) return + released = true + resolve() + } + }) + return {promise, release} +} + +function isValidIdempotencyKey(key) { + return ( + typeof key === 'string' && + key.length > 0 && + key.length <= MAX_KEY_LENGTH && + SAFE_KEY_PATTERN.test(key) + ) +} + +function buildCacheKey(prefix, req, idempotencyKey) { + const method = req.method + const url = req.url || req.originalUrl + const keyMaterial = `${method}:${url}:${idempotencyKey}` + return `${prefix}${hashSha256(keyMaterial)}` +} + +function isValidCachedResponse(value) { + return ( + value && + typeof value === 'object' && + value.version === 1 && + typeof value.status === 'number' && + value.status >= 200 && + value.status < 300 && + typeof value.cachedAt === 'number' && + value.body !== undefined && + value.body !== null + ) +} + +function replayResponse(res, cachedResponse) { + if (res.headersSent) { + throw new Error('IdempotencyMiddleware - Response headers already sent') + } + + res.statusCode = cachedResponse.status + res.setHeader('X-Idempotency-Status', 'hit') + + const headers = cachedResponse.headers || {} + for (const [name, value] of Object.entries(headers)) { + if (HOP_BY_HOP_HEADERS.has(name.toLowerCase())) { + continue } + res.setHeader(name, value) } + + res.end(deserializeBody(cachedResponse.body)) +} + +function serializeBody(body) { + if (Buffer.isBuffer(body)) { + return {type: 'buffer', data: body.toString('base64')} + } + return {type: 'string', data: String(body ?? '')} +} + +function deserializeBody(body) { + if (body && typeof body === 'object') { + if (body.type === 'buffer' && typeof body.data === 'string') { + return Buffer.from(body.data, 'base64') + } + if (body.type === 'string') { + return body.data + } + } + return body +} + +function getBodyLength(body) { + if (body === undefined || body === null) { + return 0 + } + if (Buffer.isBuffer(body)) { + return body.length + } + if (typeof body === 'string') { + return Buffer.byteLength(body, 'utf8') + } + return Infinity +} + +/** + * Wraps a promise so that it rejects if it does not settle within the given timeout. + * + * @template T + * @param {Promise} promise - The promise to wrap. + * @param {number} ms - The timeout in milliseconds. + * + * @returns {Promise} A promise that resolves or rejects with the original promise, or rejects on timeout. + */ +function withTimeout(promise, ms) { + return new Promise((resolve, reject) => { + const timer = setTimeout(() => { + reject(new Error('IdempotencyMiddleware - Cache READ Timeout')) + }, ms) + + promise.then( + (value) => { + clearTimeout(timer) + resolve(value) + }, + (err) => { + clearTimeout(timer) + reject(err) + }, + ) + }) } /** diff --git a/package.json b/package.json index e51526d..37ba98c 100644 --- a/package.json +++ b/package.json @@ -8,12 +8,14 @@ "scripts": { "test": "c8 node test", "coverage": "c8 report --reporter=html", + "lint": "prettier --check *.js", "format": "prettier --write *.js" }, "files": [ "LICENSE", "README.md", - "index.js" + "index.js", + "index.d.ts" ], "keywords": [ "idempotence", @@ -34,7 +36,7 @@ "c8": "^10.1.3", "cache-manager": "^6.3.1", "cacheable": "^1.8.6", - "express": "^4.21.2", + "express": "^4.22.2", "prettier": "^3.4.2" }, "peerDependencies": { diff --git a/test.js b/test.js index db5880d..4918b30 100644 --- a/test.js +++ b/test.js @@ -11,13 +11,17 @@ function createMockCache({ getError = null, setError = null, } = {}) { + const store = new Map() return { + store, + setCalls: [], get: async (key) => { if (getError) throw getError - return getValue + return store.get(key) ?? getValue }, set: async (key, value, options) => { if (setError) throw setError + store.set(key, value) return true }, } @@ -34,9 +38,13 @@ function createMockLogger() { } // A helper to create a mock request and response -function createMockReqRes({headers = {}, statusCode = 200} = {}) { - const req = {headers} - req.method = 'POST' +function createMockReqRes({ + headers = {}, + statusCode = 200, + method = 'POST', + url = '/create', +} = {}) { + const req = {headers, method, url} // Mock response as an EventEmitter to simulate the 'end' event. const res = new EventEmitter() @@ -45,19 +53,22 @@ function createMockReqRes({headers = {}, statusCode = 200} = {}) { res.headers[name.toLowerCase()] = value } res.getHeaders = () => res.headers + res.removeHeader = (name) => { + delete res.headers[name.toLowerCase()] + } res.endCalled = false + res.endBody = undefined res.statusCode = statusCode res.end = (body) => { res.endCalled = true - // Simulate 'end' event after a tick + res.endBody = body + // Simulate 'end' event after a tick, mirroring on-http-end's payload shape. process.nextTick(() => { - // We replicate `onEnd` behavior: the middleware already attached a listener. - // The `on-http-end` library would pass payload with status and body. - const payload = { + res.emit('end', { status: res.statusCode, + headers: res.getHeaders(), data: body, - } - res.emit('end', payload) + }) }) } @@ -73,11 +84,10 @@ test('idempotencyMiddleware - no idempotency key provided', async (t) => { cache, ttl: 3600, logger, - idempotencyHeaderKey: null, }) const {req, res} = createMockReqRes() - middleware(req, res, () => { + await middleware(req, res, () => { nextCalled = true }) @@ -89,20 +99,24 @@ test('idempotencyMiddleware - no idempotency key provided', async (t) => { }) test('idempotencyMiddleware - idempotency key present and cache hit', async (t) => { - const cache = createMockCache({getValue: true}) + const cache = createMockCache() + cache.store.set(buildExpectedKey('123'), { + version: 1, + status: 201, + headers: {'content-type': 'text/plain; charset=utf-8'}, + body: {type: 'string', data: 'Resource created!'}, + cachedAt: Date.now(), + }) const logger = createMockLogger() const middleware = idempotencyMiddleware({cache, ttl: 3600, logger}) const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) let nextCalled = false - middleware(req, res, () => { + await middleware(req, res, () => { nextCalled = true }) - // sleep for 5 ms - await new Promise((resolve) => setTimeout(resolve, 5)) - assert.strictEqual( nextCalled, false, @@ -110,32 +124,33 @@ test('idempotencyMiddleware - idempotency key present and cache hit', async (t) ) assert.strictEqual( res.statusCode, - 204, - 'Should return 204 Not Modified if cache hit', + 201, + 'Should replay the original status code', + ) + assert.strictEqual( + res.endBody, + 'Resource created!', + 'Should replay the original body', ) assert.strictEqual( - res.headers['content-type'], - 'text/plain; charset=utf-8', - 'Should set Content-Type header', + res.headers['x-idempotency-status'], + 'hit', + 'Should set X-Idempotency-Status header', ) - assert.ok(res.endCalled, 'Should end response') }) test('idempotencyMiddleware - idempotency key present and cache miss', async (t) => { - const cache = createMockCache({getValue: null}) + const cache = createMockCache() const logger = createMockLogger() const middleware = idempotencyMiddleware({cache, ttl: 3600, logger}) const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) let nextCalled = false - middleware(req, res, () => { + await middleware(req, res, () => { nextCalled = true }) - // sleep for 5 ms - await new Promise((resolve) => setTimeout(resolve, 5)) - assert.ok(nextCalled, 'Next should be called if cache miss') // Simulate the actual response sending after next (like a route handler) @@ -144,10 +159,11 @@ test('idempotencyMiddleware - idempotency key present and cache miss', async (t) // Wait a tick for onEnd to trigger await new Promise(setImmediate) - // Since status code is 200 (a 2xx), it should set the cache - // We cannot directly assert this without a spy on cache.set - // However, if we simulate a set error scenario in another test, we know it tries to set. - // For now, assume success since no error is thrown. + const stored = cache.store.get(buildExpectedKey('123')) + assert.ok(stored, 'Should store a value in the cache') + assert.strictEqual(stored.version, 1) + assert.strictEqual(stored.status, 200) + assert.strictEqual(stored.body.data, 'Hello World') }) test('idempotencyMiddleware - cache read error', async (t) => { @@ -158,13 +174,10 @@ test('idempotencyMiddleware - cache read error', async (t) => { const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) let nextCalled = false - middleware(req, res, () => { + await middleware(req, res, () => { nextCalled = true }) - // sleep for 5 ms - await new Promise((resolve) => setTimeout(resolve, 5)) - assert.ok(nextCalled, 'Next should be called even on cache error') assert.deepStrictEqual(logger.errorCalls[0], [ 'IdempotencyMiddleware - Cache READ Error:', @@ -180,13 +193,10 @@ test('idempotencyMiddleware - cache write error', async (t) => { const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) let nextCalled = false - middleware(req, res, () => { + await middleware(req, res, () => { nextCalled = true }) - // sleep for 5 ms - await new Promise((resolve) => setTimeout(resolve, 5)) - assert.ok(nextCalled, 'Next should be called on cache miss') res.end('Hello World') @@ -209,21 +219,17 @@ test('idempotencyMiddleware - non-2xx response does not cache', async (t) => { statusCode: 400, }) let nextCalled = false - middleware(req, res, () => { + await middleware(req, res, () => { nextCalled = true }) - // sleep for 5 ms - await new Promise((resolve) => setTimeout(resolve, 5)) - assert.ok(nextCalled, 'Next should be called on cache miss') // End with a 4xx error res.end('Bad Request') await new Promise(setImmediate) - // Since status code is not 2xx, it should not attempt to set cache. - // We can't easily verify this without a spy; but at least no errors or logs occur. + assert.strictEqual(cache.store.size, 0, 'Cache should remain empty') assert.strictEqual(logger.errorCalls.length, 0, 'No error logs expected') }) @@ -234,17 +240,14 @@ test('idempotencyMiddleware - should skip GET method', async (t) => { const {req, res} = createMockReqRes({ headers: {'x-request-id': '123'}, + method: 'GET', }) let nextCalled = false - req.method = 'GET' - middleware(req, res, () => { + await middleware(req, res, () => { nextCalled = true }) - // sleep for 5 ms - await new Promise((resolve) => setTimeout(resolve, 5)) - assert.ok(nextCalled, 'Next should be called on GET method') }) @@ -281,6 +284,20 @@ test('idempotencyMiddleware - should trigger error ttl is not a number', async ( }) }) +test('idempotencyMiddleware - should reject ttl above maximum', async (t) => { + const cache = createMockCache() + const logger = createMockLogger() + + assert.throws( + () => idempotencyMiddleware({cache, ttl: 24 * 60 * 60 * 1000 + 1, logger}), + { + name: 'Error', + message: + 'IdempotencyMiddleware: ttl must be between 1 and 86400000 milliseconds.', + }, + ) +}) + test('idempotencyMiddleware - hashSha256 function', async (t) => { const hash = hashSha256('123') assert.strictEqual( @@ -288,3 +305,800 @@ test('idempotencyMiddleware - hashSha256 function', async (t) => { 'a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae3', ) }) + +test('idempotencyMiddleware - rejects unsafe idempotency keys', async (t) => { + const cache = createMockCache() + const logger = createMockLogger() + const middleware = idempotencyMiddleware({cache, ttl: 3600, logger}) + + const {req, res} = createMockReqRes({ + headers: {'x-request-id': 'key with spaces'}, + }) + let nextCalled = false + await middleware(req, res, () => { + nextCalled = true + }) + + assert.ok(nextCalled, 'Next should be called for unsafe keys') + assert.strictEqual(cache.store.size, 0) +}) + +test('idempotencyMiddleware - rejects idempotency keys that are too long', async (t) => { + const cache = createMockCache() + const logger = createMockLogger() + const middleware = idempotencyMiddleware({cache, ttl: 3600, logger}) + + const {req, res} = createMockReqRes({ + headers: {'x-request-id': 'x'.repeat(129)}, + }) + let nextCalled = false + await middleware(req, res, () => { + nextCalled = true + }) + + assert.ok(nextCalled, 'Next should be called for overlong keys') +}) + +test('idempotencyMiddleware - extractor errors are handled gracefully', async (t) => { + const cache = createMockCache() + const logger = createMockLogger() + const extractorError = new Error('extractor failed') + const middleware = idempotencyMiddleware({ + cache, + ttl: 3600, + logger, + idempotencyKeyExtractor: () => { + throw extractorError + }, + }) + + const {req, res} = createMockReqRes() + let nextCalled = false + await middleware(req, res, () => { + nextCalled = true + }) + + assert.ok(nextCalled, 'Next should be called when extractor throws') + assert.deepStrictEqual(logger.errorCalls[0], [ + 'IdempotencyMiddleware - Extractor Error:', + extractorError, + ]) +}) + +test('idempotencyMiddleware - keyPrefix option is honored', async (t) => { + const cache = createMockCache() + const middleware = idempotencyMiddleware({ + cache, + ttl: 3600, + keyPrefix: 'custom-prefix-', + }) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + await middleware(req, res, () => { + res.end('Hello') + }) + await new Promise(setImmediate) + + const keys = Array.from(cache.store.keys()) + assert.strictEqual(keys.length, 1) + assert.ok( + keys[0].startsWith('custom-prefix-'), + 'Cache key should use the configured prefix', + ) +}) + +test('idempotencyMiddleware - concurrent duplicate requests are deduplicated', async (t) => { + const cache = createMockCache() + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + let handlerCalls = 0 + const responses = [] + + function runRequest() { + return new Promise((resolve) => { + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + + middleware(req, res, () => { + handlerCalls++ + // Simulate some asynchronous work + setImmediate(() => { + res.statusCode = 201 + res.end('created') + }) + }) + + res.on('end', () => { + responses.push({status: res.statusCode, body: res.endBody}) + resolve() + }) + }) + } + + await Promise.all([runRequest(), runRequest()]) + + assert.strictEqual(handlerCalls, 1, 'Handler should only run once') + assert.strictEqual(responses.length, 2) + assert.strictEqual(responses[0].status, 201) + assert.strictEqual(responses[1].status, 201) + assert.strictEqual(responses[0].body, 'created') + assert.strictEqual(responses[1].body, 'created') +}) + +test('idempotencyMiddleware - cache key is scoped by method and url', async (t) => { + const cache = createMockCache() + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req: req1, res: res1} = createMockReqRes({ + headers: {'x-request-id': '123'}, + url: '/orders', + }) + await middleware(req1, res1, () => { + res1.end('orders') + }) + await new Promise(setImmediate) + + const {req: req2, res: res2} = createMockReqRes({ + headers: {'x-request-id': '123'}, + url: '/invoices', + }) + let next2Called = false + await middleware(req2, res2, () => { + next2Called = true + res2.end('invoices') + }) + await new Promise(setImmediate) + + assert.ok(next2Called, 'Different URL should not share cache key') + assert.strictEqual(cache.store.size, 2) +}) + +test('idempotencyMiddleware - invalid cache values are ignored', async (t) => { + const cache = createMockCache() + cache.store.set(buildExpectedKey('123'), {version: 0, status: 200}) + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + let nextCalled = false + await middleware(req, res, () => { + nextCalled = true + res.end('ok') + }) + + assert.ok(nextCalled, 'Next should be called for invalid cached values') +}) + +test('idempotencyMiddleware - validates cache instance presence', async (t) => { + assert.throws(() => idempotencyMiddleware({ttl: 3600}), { + message: + 'IdempotencyMiddleware: A valid cache instance with .get and .set methods is required.', + }) +}) + +test('idempotencyMiddleware - validates ttl options', async (t) => { + const cache = createMockCache() + + assert.throws(() => idempotencyMiddleware({cache, ttl: Infinity}), { + message: + 'IdempotencyMiddleware: A positive numeric ttl (in milliseconds) is required.', + }) + assert.throws(() => idempotencyMiddleware({cache, ttl: 0}), { + message: + 'IdempotencyMiddleware: A positive numeric ttl (in milliseconds) is required.', + }) + assert.throws(() => idempotencyMiddleware({cache, ttl: -1}), { + message: + 'IdempotencyMiddleware: A positive numeric ttl (in milliseconds) is required.', + }) +}) + +test('idempotencyMiddleware - validates keyPrefix option', async (t) => { + const cache = createMockCache() + + assert.throws( + () => idempotencyMiddleware({cache, ttl: 3600, keyPrefix: ''}), + { + message: 'IdempotencyMiddleware: keyPrefix must be a non-empty string.', + }, + ) + assert.throws( + () => idempotencyMiddleware({cache, ttl: 3600, keyPrefix: 123}), + { + message: 'IdempotencyMiddleware: keyPrefix must be a non-empty string.', + }, + ) +}) + +test('idempotencyMiddleware - validates maxResponseSize option', async (t) => { + const cache = createMockCache() + + assert.throws( + () => idempotencyMiddleware({cache, ttl: 3600, maxResponseSize: 0}), + { + message: + 'IdempotencyMiddleware: maxResponseSize must be a positive number.', + }, + ) + assert.throws( + () => idempotencyMiddleware({cache, ttl: 3600, maxResponseSize: -1}), + { + message: + 'IdempotencyMiddleware: maxResponseSize must be a positive number.', + }, + ) + assert.throws( + () => idempotencyMiddleware({cache, ttl: 3600, maxResponseSize: Infinity}), + { + message: + 'IdempotencyMiddleware: maxResponseSize must be a positive number.', + }, + ) + assert.throws( + () => idempotencyMiddleware({cache, ttl: 3600, maxResponseSize: '1MB'}), + { + message: + 'IdempotencyMiddleware: maxResponseSize must be a positive number.', + }, + ) +}) + +test('idempotencyMiddleware - empty idempotency key is skipped', async (t) => { + const cache = createMockCache() + const middleware = idempotencyMiddleware({ + cache, + ttl: 3600, + idempotencyKeyExtractor: () => '', + }) + + const {req, res} = createMockReqRes() + let nextCalled = false + await middleware(req, res, () => { + nextCalled = true + }) + + assert.ok(nextCalled) +}) + +test('idempotencyMiddleware - array header is skipped', async (t) => { + const cache = createMockCache() + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': ['123']}}) + let nextCalled = false + await middleware(req, res, () => { + nextCalled = true + }) + + assert.ok(nextCalled) +}) + +test('idempotencyMiddleware - unexpected errors are passed to next', async (t) => { + const cache = createMockCache() + const logger = createMockLogger() + const middleware = idempotencyMiddleware({cache, ttl: 3600, logger}) + + const req = { + get method() { + throw new Error('method access failed') + }, + headers: {}, + } + const res = new EventEmitter() + let nextArg + await middleware(req, res, (err) => { + nextArg = err + }) + + assert.ok(nextArg instanceof Error) + assert.strictEqual(nextArg.message, 'method access failed') + assert.strictEqual( + logger.errorCalls[0][0], + 'IdempotencyMiddleware - Unexpected Error:', + ) +}) + +test('idempotencyMiddleware - hop-by-hop headers are not replayed', async (t) => { + const cache = createMockCache() + cache.store.set(buildExpectedKey('123'), { + version: 1, + status: 200, + headers: { + 'content-type': 'text/plain', + 'content-length': '17', + connection: 'keep-alive', + date: 'Mon, 01 Jan 2024 00:00:00 GMT', + }, + body: {type: 'string', data: 'Resource created!'}, + cachedAt: Date.now(), + }) + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + await middleware(req, res, () => {}) + + assert.strictEqual(res.statusCode, 200) + assert.strictEqual(res.headers['content-type'], 'text/plain') + assert.strictEqual(res.headers['content-length'], undefined) + assert.strictEqual(res.headers.connection, undefined) + assert.strictEqual(res.headers.date, undefined) +}) + +test('idempotencyMiddleware - buffers are cached and replayed', async (t) => { + const cache = createMockCache() + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + await middleware(req, res, () => { + res.end(Buffer.from('binary data')) + }) + await new Promise(setImmediate) + + const stored = cache.store.get(buildExpectedKey('123')) + assert.strictEqual(stored.body.type, 'buffer') + assert.strictEqual( + stored.body.data, + Buffer.from('binary data').toString('base64'), + ) + + const {req: req2, res: res2} = createMockReqRes({ + headers: {'x-request-id': '123'}, + }) + await middleware(req2, res2, () => {}) + assert.ok(Buffer.isBuffer(res2.endBody)) + assert.strictEqual(res2.endBody.toString(), 'binary data') +}) + +test('idempotencyMiddleware - responses larger than maxResponseSize are not cached', async (t) => { + const cache = createMockCache() + const middleware = idempotencyMiddleware({ + cache, + ttl: 3600, + maxResponseSize: 5, + }) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + await middleware(req, res, () => { + res.end('Hello World') + }) + await new Promise(setImmediate) + + assert.strictEqual(cache.store.size, 0) +}) + +test('idempotencyMiddleware - empty responses are cached', async (t) => { + const cache = createMockCache() + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + await middleware(req, res, () => { + res.end() + }) + await new Promise(setImmediate) + + const stored = cache.store.get(buildExpectedKey('123')) + assert.strictEqual(stored.body.type, 'string') + assert.strictEqual(stored.body.data, '') +}) + +test('idempotencyMiddleware - unknown body type passes through deserialization', async (t) => { + const cache = createMockCache() + cache.store.set(buildExpectedKey('123'), { + version: 1, + status: 200, + headers: {}, + body: {type: 'unknown', data: 'foo'}, + cachedAt: Date.now(), + }) + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + await middleware(req, res, () => {}) + + assert.deepStrictEqual(res.endBody, {type: 'unknown', data: 'foo'}) +}) + +test('idempotencyMiddleware - buffer body with invalid data passes through', async (t) => { + const cache = createMockCache() + cache.store.set(buildExpectedKey('123'), { + version: 1, + status: 200, + headers: {}, + body: {type: 'buffer', data: 123}, + cachedAt: Date.now(), + }) + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + await middleware(req, res, () => {}) + + assert.deepStrictEqual(res.endBody, {type: 'buffer', data: 123}) +}) + +test('idempotencyMiddleware - primitive body passes through deserialization', async (t) => { + const cache = createMockCache() + cache.store.set(buildExpectedKey('123'), { + version: 1, + status: 200, + headers: {}, + body: 'raw body', + cachedAt: Date.now(), + }) + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + await middleware(req, res, () => {}) + + assert.strictEqual(res.endBody, 'raw body') +}) + +test('idempotencyMiddleware - falsy string body is replayed', async (t) => { + const cache = createMockCache() + cache.store.set(buildExpectedKey('123'), { + version: 1, + status: 200, + headers: {}, + body: '', + cachedAt: Date.now(), + }) + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + await middleware(req, res, () => {}) + + assert.strictEqual(res.endBody, '') +}) + +test('idempotencyMiddleware - ignores cached responses with non-2xx status', async (t) => { + const cache = createMockCache() + cache.store.set(buildExpectedKey('123'), { + version: 1, + status: 199, + headers: {}, + body: {type: 'string', data: 'ok'}, + cachedAt: Date.now(), + }) + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + let nextCalled = false + await middleware(req, res, () => { + nextCalled = true + res.end('ok') + }) + + assert.ok(nextCalled) +}) + +test('idempotencyMiddleware - ignores cached responses with 3xx status', async (t) => { + const cache = createMockCache() + cache.store.set(buildExpectedKey('123'), { + version: 1, + status: 301, + headers: {}, + body: {type: 'string', data: 'ok'}, + cachedAt: Date.now(), + }) + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + let nextCalled = false + await middleware(req, res, () => { + nextCalled = true + res.end('ok') + }) + + assert.ok(nextCalled) +}) + +test('idempotencyMiddleware - ignores cached responses with non-numeric status', async (t) => { + const cache = createMockCache() + cache.store.set(buildExpectedKey('123'), { + version: 1, + status: '200', + headers: {}, + body: {type: 'string', data: 'ok'}, + cachedAt: Date.now(), + }) + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + let nextCalled = false + await middleware(req, res, () => { + nextCalled = true + res.end('ok') + }) + + assert.ok(nextCalled) +}) + +test('idempotencyMiddleware - ignores cached responses with missing cachedAt', async (t) => { + const cache = createMockCache() + cache.store.set(buildExpectedKey('123'), { + version: 1, + status: 200, + headers: {}, + body: {type: 'string', data: 'ok'}, + }) + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + let nextCalled = false + await middleware(req, res, () => { + nextCalled = true + res.end('ok') + }) + + assert.ok(nextCalled) +}) + +test('idempotencyMiddleware - ignores cached responses with null body', async (t) => { + const cache = createMockCache() + cache.store.set(buildExpectedKey('123'), { + version: 1, + status: 200, + headers: {}, + body: null, + cachedAt: Date.now(), + }) + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + let nextCalled = false + await middleware(req, res, () => { + nextCalled = true + res.end('ok') + }) + + assert.ok(nextCalled) +}) + +test('idempotencyMiddleware - ignores cached string values', async (t) => { + const cache = createMockCache() + cache.store.set(buildExpectedKey('123'), 'cached value') + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + let nextCalled = false + await middleware(req, res, () => { + nextCalled = true + res.end('ok') + }) + + assert.ok(nextCalled) +}) + +test('idempotencyMiddleware - ignores cached responses with undefined headers', async (t) => { + const cache = createMockCache() + cache.store.set(buildExpectedKey('123'), { + version: 1, + status: 200, + headers: undefined, + body: {type: 'string', data: 'ok'}, + cachedAt: Date.now(), + }) + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + await middleware(req, res, () => {}) + + assert.strictEqual(res.statusCode, 200) + assert.strictEqual(res.endBody, 'ok') +}) + +test('idempotencyMiddleware - falls back to originalUrl when url is missing', async (t) => { + const cache = createMockCache() + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + delete req.url + req.originalUrl = '/original' + await middleware(req, res, () => { + res.end('ok') + }) + await new Promise(setImmediate) + + const keys = Array.from(cache.store.keys()) + assert.strictEqual(keys.length, 1) + assert.ok(keys[0].startsWith('idemp-key-')) +}) + +test('idempotencyMiddleware - three concurrent requests are deduplicated', async (t) => { + const cache = createMockCache() + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + let handlerCalls = 0 + const responses = [] + + function runRequest() { + return new Promise((resolve) => { + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + + middleware(req, res, () => { + handlerCalls++ + setImmediate(() => { + res.statusCode = 201 + res.end('created') + }) + }) + + res.on('end', () => { + responses.push({status: res.statusCode, body: res.endBody}) + resolve() + }) + }) + } + + await Promise.all([runRequest(), runRequest(), runRequest()]) + + assert.strictEqual(handlerCalls, 1) + assert.strictEqual(responses.length, 3) + for (const response of responses) { + assert.strictEqual(response.status, 201) + assert.strictEqual(response.body, 'created') + } +}) + +test('idempotencyMiddleware - application headers are captured and replayed', async (t) => { + const cache = createMockCache() + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + await middleware(req, res, () => { + res.setHeader('content-type', 'application/json') + res.setHeader('x-custom', 'foo') + res.end('Hello') + }) + await new Promise(setImmediate) + + const stored = cache.store.get(buildExpectedKey('123')) + assert.strictEqual(stored.headers['content-type'], 'application/json') + assert.strictEqual(stored.headers['x-custom'], 'foo') + + const {req: req2, res: res2} = createMockReqRes({ + headers: {'x-request-id': '123'}, + }) + await middleware(req2, res2, () => {}) + assert.strictEqual(res2.headers['content-type'], 'application/json') + assert.strictEqual(res2.headers['x-custom'], 'foo') + assert.strictEqual(res2.endBody, 'Hello') +}) + +test('idempotencyMiddleware - validates cacheTimeout option', async (t) => { + const cache = createMockCache() + + assert.throws( + () => idempotencyMiddleware({cache, ttl: 3600, cacheTimeout: 0}), + { + message: 'IdempotencyMiddleware: cacheTimeout must be a positive number.', + }, + ) + assert.throws( + () => idempotencyMiddleware({cache, ttl: 3600, cacheTimeout: -1}), + { + message: 'IdempotencyMiddleware: cacheTimeout must be a positive number.', + }, + ) + assert.throws( + () => idempotencyMiddleware({cache, ttl: 3600, cacheTimeout: Infinity}), + { + message: 'IdempotencyMiddleware: cacheTimeout must be a positive number.', + }, + ) + assert.throws( + () => idempotencyMiddleware({cache, ttl: 3600, cacheTimeout: '1s'}), + { + message: 'IdempotencyMiddleware: cacheTimeout must be a positive number.', + }, + ) +}) + +test('idempotencyMiddleware - cache.get timeout releases lock and calls next', async (t) => { + const logger = createMockLogger() + let getCalls = 0 + const cache = createMockCache() + cache.get = async (key) => { + getCalls++ + if (getCalls === 1) return new Promise(() => {}) + return null + } + const middleware = idempotencyMiddleware({ + cache, + ttl: 3600, + logger, + cacheTimeout: 10, + }) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + let nextCalled = false + await middleware(req, res, () => { + nextCalled = true + }) + + assert.ok(nextCalled, 'Next should be called when cache.get times out') + assert.strictEqual( + logger.errorCalls[0][0], + 'IdempotencyMiddleware - Cache READ Error:', + ) + assert.ok(logger.errorCalls[0][1].message.includes('Cache READ Timeout')) + + // A follow-up request with the same key should be able to acquire a new lock. + const {req: req2, res: res2} = createMockReqRes({ + headers: {'x-request-id': '123'}, + }) + let next2Called = false + await middleware(req2, res2, () => { + next2Called = true + }) + assert.ok(next2Called, 'Lock should be released after timeout') +}) + +test('idempotencyMiddleware - unknown body types are not cached when maxResponseSize is set', async (t) => { + const cache = createMockCache() + const middleware = idempotencyMiddleware({ + cache, + ttl: 3600, + maxResponseSize: 1000, + }) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + await middleware(req, res, () => { + res.end({foo: 'bar'}) + }) + await new Promise(setImmediate) + + assert.strictEqual( + cache.store.size, + 0, + 'Unknown body types should not be cached', + ) +}) + +test('idempotencyMiddleware - replay aborts if response headers already sent', async (t) => { + const cache = createMockCache() + cache.store.set(buildExpectedKey('123'), { + version: 1, + status: 200, + headers: {}, + body: {type: 'string', data: 'ok'}, + cachedAt: Date.now(), + }) + const logger = createMockLogger() + const middleware = idempotencyMiddleware({cache, ttl: 3600, logger}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + res.headersSent = true + let nextError + await middleware(req, res, (err) => { + nextError = err + }) + + assert.ok(nextError instanceof Error) + assert.strictEqual( + nextError.message, + 'IdempotencyMiddleware - Response headers already sent', + ) + assert.strictEqual(res.endCalled, false) +}) + +test('idempotencyMiddleware - close event does not leak or double-release lock', async (t) => { + const cache = createMockCache() + const middleware = idempotencyMiddleware({cache, ttl: 3600}) + + const {req, res} = createMockReqRes({headers: {'x-request-id': '123'}}) + let nextCalled = false + await middleware(req, res, () => { + nextCalled = true + }) + assert.ok(nextCalled) + + // Simulate a client disconnect before the response finishes. + res.emit('close') + // Then finish the response as the handler eventually would. + res.end('ok') + await new Promise(setImmediate) + + const stored = cache.store.get(buildExpectedKey('123')) + assert.ok(stored, 'Response should still be cached after close + end') +}) + +function buildExpectedKey(idempotencyKey, method = 'POST', url = '/create') { + const keyMaterial = `${method}:${url}:${idempotencyKey}` + return `idemp-key-${hashSha256(keyMaterial)}` +}