Skip to content

Commit b961b78

Browse files
committed
fix(event-handler): threshold limit for compression not respected when content-length not set
1 parent 80d62bd commit b961b78

File tree

5 files changed

+95
-6
lines changed

5 files changed

+95
-6
lines changed

packages/event-handler/src/http/Router.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import type {
2424
ErrorHandler,
2525
ErrorResolveOptions,
2626
HttpMethod,
27+
HttpResolveOptions,
2728
HttpRouteOptions,
2829
HttpRouterOptions,
2930
Middleware,
@@ -221,7 +222,7 @@ class Router {
221222
async #resolve(
222223
event: unknown,
223224
context: Context,
224-
options?: ResolveOptions
225+
options?: HttpResolveOptions
225226
): Promise<RequestContext> {
226227
if (
227228
!isAPIGatewayProxyEventV1(event) &&
@@ -248,7 +249,12 @@ class Router {
248249
event,
249250
context,
250251
req: new Request('https://invalid'),
251-
res: new Response('', { status: HttpStatusCodes.METHOD_NOT_ALLOWED }),
252+
res: new Response(null, {
253+
status: HttpStatusCodes.METHOD_NOT_ALLOWED,
254+
...(options?.isHttpStreaming && {
255+
headers: { 'transfer-encoding': 'chunked' },
256+
}),
257+
}),
252258
params: {},
253259
responseType,
254260
};
@@ -262,7 +268,12 @@ class Router {
262268
req,
263269
// this response should be overwritten by the handler, if it isn't
264270
// it means something went wrong with the middleware chain
265-
res: new Response('', { status: HttpStatusCodes.INTERNAL_SERVER_ERROR }),
271+
res: new Response('', {
272+
status: HttpStatusCodes.INTERNAL_SERVER_ERROR,
273+
...(options?.isHttpStreaming && {
274+
headers: { 'transfer-encoding': 'chunked' },
275+
}),
276+
}),
266277
params: {},
267278
responseType,
268279
};
@@ -393,7 +404,10 @@ class Router {
393404
context: Context,
394405
options: ResolveStreamOptions
395406
): Promise<void> {
396-
const reqCtx = await this.#resolve(event, context, options);
407+
const reqCtx = await this.#resolve(event, context, {
408+
...options,
409+
isHttpStreaming: true,
410+
});
397411
await this.#streamHandlerResponse(reqCtx, options.responseStream);
398412
}
399413

packages/event-handler/src/http/middleware/compress.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,14 @@ const compress = (options?: CompressionOptions): Middleware => {
7070
return async ({ reqCtx, next }) => {
7171
await next();
7272

73+
const transferEncoding = reqCtx.res.headers.get('transfer-encoding');
74+
if (transferEncoding === 'chunked') return;
75+
76+
if (reqCtx.res.body !== null && !reqCtx.res.headers.has('content-length')) {
77+
const body = await reqCtx.res.clone().arrayBuffer();
78+
reqCtx.res.headers.set('content-length', body.byteLength.toString());
79+
}
80+
7381
if (!shouldCompress(reqCtx.req, reqCtx.res, preferredEncoding, threshold)) {
7482
return;
7583
}

packages/event-handler/src/types/http.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ type RequestContext = {
3434
isBase64Encoded?: boolean;
3535
};
3636

37+
type HttpResolveOptions = ResolveOptions & { isHttpStreaming?: boolean };
38+
3739
type ErrorResolveOptions = RequestContext & ResolveOptions;
3840

3941
type ErrorHandler<T extends Error = Error> = (
@@ -263,6 +265,7 @@ export type {
263265
ErrorHandler,
264266
ErrorResolveOptions,
265267
HandlerResponse,
268+
HttpResolveOptions,
266269
HttpStatusCode,
267270
HttpMethod,
268271
Middleware,

packages/event-handler/tests/unit/http/Router/streaming.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,4 +357,19 @@ describe.each([
357357
expect(result.statusCode).toBe(200);
358358
expect(JSON.parse(result.body)).toEqual({ message: 'duplex stream body' });
359359
});
360+
361+
it('handles invalid HTTP method', async () => {
362+
// Prepare
363+
const app = new Router();
364+
const handler = streamify(app);
365+
const responseStream = new ResponseStream();
366+
const event = createEvent('/test', 'TRACE');
367+
368+
// Act
369+
const result = await handler(event, responseStream, context);
370+
371+
// Assess
372+
expect(result.statusCode).toBe(405);
373+
expect(result.body).toBe('');
374+
});
360375
});

packages/event-handler/tests/unit/http/middleware/compress.test.ts

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
1-
import { gzipSync } from 'node:zlib';
1+
import { deflateSync, gzipSync } from 'node:zlib';
22
import context from '@aws-lambda-powertools/testing-utils/context';
33
import { beforeEach, describe, expect, it } from 'vitest';
4+
import { streamify } from '../../../../src/http/index.js';
45
import { compress } from '../../../../src/http/middleware/index.js';
56
import { Router } from '../../../../src/http/Router.js';
6-
import { createSettingHeadersMiddleware, createTestEvent } from '../helpers.js';
7+
import {
8+
createSettingHeadersMiddleware,
9+
createTestEvent,
10+
ResponseStream,
11+
} from '../helpers.js';
712

813
describe('Compress Middleware', () => {
914
const event = createTestEvent('/test', 'GET');
@@ -62,6 +67,24 @@ describe('Compress Middleware', () => {
6267
expect(result.isBase64Encoded).toBe(false);
6368
});
6469

70+
it('skips compression when content is below threshold and content-length is not set', async () => {
71+
// Prepare
72+
const application = new Router();
73+
const smallBody = { message: 'Small' };
74+
75+
application.use(compress({ threshold: 100 }));
76+
application.get('/test', () => {
77+
return smallBody;
78+
});
79+
80+
// Act
81+
const result = await application.resolve(event, context);
82+
83+
// Assess
84+
expect(result.headers?.['content-encoding']).toBeUndefined();
85+
expect(result.isBase64Encoded).toBe(false);
86+
});
87+
6588
it('skips compression for HEAD requests', async () => {
6689
// Prepare
6790
const headEvent = createTestEvent('/test', 'HEAD');
@@ -104,6 +127,7 @@ describe('Compress Middleware', () => {
104127
// Assess
105128
expect(result.headers?.['content-encoding']).toEqual('gzip');
106129
expect(result.isBase64Encoded).toBe(true);
130+
expect(result.body).toBe(gzipSync(JSON.stringify(body)).toString('base64'));
107131
});
108132

109133
it('skips compression when cache-control no-transform is set', async () => {
@@ -155,6 +179,9 @@ describe('Compress Middleware', () => {
155179
// Assess
156180
expect(result.headers?.['content-encoding']).toBe('deflate');
157181
expect(result.isBase64Encoded).toBe(true);
182+
expect(result.body).toBe(
183+
deflateSync(JSON.stringify(body)).toString('base64')
184+
);
158185
});
159186

160187
it('does not compress if Accept-Encoding is set to identity', async () => {
@@ -173,4 +200,26 @@ describe('Compress Middleware', () => {
173200
expect(result.headers?.['content-encoding']).toBeUndefined();
174201
expect(result.isBase64Encoded).toBe(false);
175202
});
203+
204+
it('skips compression and content-length in streaming mode', async () => {
205+
// Prepare
206+
const application = new Router();
207+
application.use(compress());
208+
application.get('/test', () => body);
209+
210+
const handler = streamify(application);
211+
const responseStream = new ResponseStream();
212+
213+
// Act
214+
const result = await handler(event, responseStream, context);
215+
216+
// Assess
217+
expect(result.statusCode).toBe(200);
218+
expect(result.headers['content-encoding']).toBeUndefined();
219+
expect(result.headers['content-length']).toBeUndefined();
220+
expect(result.body).toBe(JSON.stringify(body));
221+
expect(result.body).not.toBe(
222+
gzipSync(JSON.stringify(body)).toString('base64')
223+
);
224+
});
176225
});

0 commit comments

Comments
 (0)