-
Notifications
You must be signed in to change notification settings - Fork 179
Description
Expected Behavior
The compress middleware should only compress responses that exceed the configured threshold (default: 1024 bytes, configurable via options). When a response is below the threshold, it should NOT be compressed, regardless of whether the content-length header is present.
For example, with a threshold of 100 bytes:
- A 20-byte response should NOT be compressed
- A 200-byte response SHOULD be compressed
Current Behavior
The compression middleware always compresses responses when the content-length header is not set, completely ignoring the threshold setting.
Since JSON responses generated by the Event Handler do NOT have content-length headers set by default, all JSON responses are compressed regardless of size, defeating the purpose of the threshold configuration.
In packages/event-handler/src/rest/middleware/compress.ts line 109 we have: (!contentLength || Number(contentLength) > threshold)
This condition means:
- If
content-lengthis NOT set → compress (regardless of threshold) - If
content-lengthIS set AND greater than threshold → compress
The logic should only compress when we can determine the size AND it exceeds the threshold.
Code snippet
Failing E2E test:
// packages/event-handler/tests/e2e/restEventHandler.test.ts
it('does not compress small responses below threshold', async () => {
const response = await fetch(`${apiUrl}/compress/small`, {
headers: { 'Accept-Encoding': 'gzip' },
});
const data = await response.json();
expect(response.status).toBe(200);
expect(data.message).toBe('Small');
// Small response (~20 bytes) is below 100 byte threshold, should not be compressed
expect(response.headers.get('content-encoding')).toBeNull(); // FAILS - gets 'gzip'
});Handler code:
// packages/event-handler/tests/e2e/routers/compressRouter.ts
const compressRouter = new Router();
compressRouter.use(compress({ threshold: 100 })); // 100 byte threshold
compressRouter.get('/small', () => ({
message: 'Small', // ~20 bytes when stringified
}));Failing unit test demonstrating the bug:
// Add to packages/event-handler/tests/unit/rest/middleware/compress.test.ts after line 63
it('skips compression when content is below threshold and content-length is not set', async
() => {
// Prepare
// This test simulates real-world usage where JSON responses don't have
// content-length headers set. The middleware should NOT compress small responses
// even when content-length is missing.
const application = new Router();
const smallBody = { message: 'Small' }; // ~20 bytes when stringified
application.use(compress({ threshold: 100 })); // 100 byte threshold
application.get('/test', () => {
return smallBody;
});
// Act
const result = await application.resolve(event, context);
// Assess
// BUG: Currently compresses because (!contentLength || Number(contentLength) > threshold)
// evaluates to true when contentLength is undefined
expect(result.headers?.['content-encoding']).toBeUndefined();
expect(result.isBase64Encoded).toBe(false);
});Steps to Reproduce
- Create a Router with the compress middleware using a threshold
- Add a route that returns a small JSON object (below threshold)
- Make a request with
Accept-Encoding: gzipheader - Observe that the response is compressed despite being below threshold
Alternatively, paste the unit test shown above in the codebase and observe it fail.
Possible Solution
There are several approaches to fix this, and I'd like to invite discussion on the best solution:
Option 1: Calculate content length when missing
Before checking the threshold, calculate the content length from the response body:
const shouldCompress = (
request: Request,
response: Response,
preferredEncoding: NonNullable<CompressionOptions['encoding']>,
threshold: NonNullable<CompressionOptions['threshold']>
): response is Response & { body: NonNullable<Response['body']> } => {
// ... existing code ...
let contentLength = response.headers.get('content-length');
// Calculate content length if not set
if (!contentLength && response.body) {
// For JSON responses, we'd need to stringify to get the size
// This might require buffering the response
}
return (
shouldEncode &&
!isEncodedOrChunked &&
request.method !== 'HEAD' &&
contentLength && Number(contentLength) > threshold && // Only compress if we know the size
(!cacheControl || !CACHE_CONTROL_NO_TRANSFORM_REGEX.test(cacheControl)) &&
response.body !== null
);
};Option 2: Skip compression when content-length is unknown
Conservative approach - don't compress if we can't determine the size:
return (
shouldEncode &&
!isEncodedOrChunked &&
request.method !== 'HEAD' &&
contentLength && Number(contentLength) > threshold && // Require content-length to be set
(!cacheControl || !CACHE_CONTROL_NO_TRANSFORM_REGEX.test(cacheControl)) &&
response.body !== null
);Option 3: Set content-length in Router when creating JSON responses
Modify the Router to always set Content-Length headers:
// In Router.ts where Response is created
const responseBody = JSON.stringify(body);
return new Response(responseBody, {
status,
headers: {
'Content-Type': 'application/json',
'Content-Length': String(new TextEncoder().encode(responseBody).length)
},
});Trade-offs:
- Option 1: Most accurate but requires buffering responses (performance impact)
- Option 2: Safest but won't compress any JSON responses without manual header setting
- Option 3: Requires changes to Router but most transparent for middleware
Do we know how the Python Event Handler does this?
Powertools for AWS Lambda (TypeScript) version
latest
AWS Lambda function runtime
22.x
Packaging format used
npm
Execution logs
Metadata
Metadata
Assignees
Labels
Type
Projects
Status