Skip to content

Commit

Permalink
fix: Specify HTTP request body limits consistent with RAMF spec (#119)
Browse files Browse the repository at this point in the history
  • Loading branch information
gnarea authored Aug 10, 2023
1 parent a73e1a2 commit 96e6854
Show file tree
Hide file tree
Showing 10 changed files with 90 additions and 17 deletions.
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"@relaycorp/awala-keystore-mongodb": "^1.1.11",
"@relaycorp/cloudevents-transport": "^1.3.0",
"@relaycorp/pino-cloud": "^1.0.28",
"@relaycorp/relaynet-core": "^1.86.2",
"@relaycorp/relaynet-core": "^1.87.0",
"@relaycorp/relaynet-pohttp": "^1.7.61",
"@relaycorp/relaynet-testing": "^2.2.28",
"@relaycorp/webcrypto-kms": "^1.5.16",
Expand Down
34 changes: 34 additions & 0 deletions src/client/server.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { Connection } from 'mongoose';
import { generatePDACertificationPath } from '@relaycorp/relaynet-testing';
import {
CertificationPath,
MAX_SDU_PLAINTEXT_LENGTH,
Parcel,
PrivateEndpointConnParams,
ServiceMessage,
Expand Down Expand Up @@ -109,6 +110,39 @@ describe('makePohttpClient', () => {
});
});

describe('Request body', () => {
test('should be accepted if it is as big than biggest service message', async () => {
const event = new CloudEvent({
id: CE_ID,
source: server.activeEndpoint.id,
type: OUTGOING_SERVICE_MESSAGE_TYPE,
datacontenttype: SERVICE_MESSAGE_CONTENT_TYPE,
expiry: formatISO(addDays(Date.now(), 1)),
data: Buffer.from('a'.repeat(MAX_SDU_PLAINTEXT_LENGTH)),
});

const response = await postEvent(event, server);

expect(response.statusCode).toBe(HTTP_STATUS_CODES.BAD_REQUEST);
});

test('should be refused if bigger than biggest service message', async () => {
const event = new CloudEvent({
id: CE_ID,
source: server.activeEndpoint.id,
subject: 'the subject',
type: OUTGOING_SERVICE_MESSAGE_TYPE,
datacontenttype: SERVICE_MESSAGE_CONTENT_TYPE,
expiry: formatISO(addDays(Date.now(), 1)),
data: Buffer.from('a'.repeat(MAX_SDU_PLAINTEXT_LENGTH + 1)),
});

const response = await postEvent(event, server);

expect(response.statusCode).toBe(HTTP_STATUS_CODES.CONTENT_TOO_LARGE);
});
});

describe('POST', () => {
const expiry = addDays(Date.now(), 5);
let peerSessionPrivateKey: CryptoKey;
Expand Down
4 changes: 2 additions & 2 deletions src/client/server.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { makeReceiver } from '@relaycorp/cloudevents-transport';
import { Parcel, ServiceMessage } from '@relaycorp/relaynet-core';
import { MAX_SDU_PLAINTEXT_LENGTH, Parcel, ServiceMessage } from '@relaycorp/relaynet-core';
import {
deliverParcel,
PoHTTPClientBindingError,
Expand Down Expand Up @@ -137,5 +137,5 @@ export async function makePohttpClientPlugin(server: FastifyInstance): Promise<v
}

export async function makePohttpClient(logger?: BaseLogger): Promise<FastifyInstance> {
return makeFastify(makePohttpClientPlugin, logger);
return makeFastify(makePohttpClientPlugin, { logger, bodyLimit: MAX_SDU_PLAINTEXT_LENGTH });
}
20 changes: 18 additions & 2 deletions src/server/server.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { jest } from '@jest/globals';
import { MAX_RAMF_MESSAGE_LENGTH } from '@relaycorp/relaynet-core';
import pino from 'pino';
import type { FastifyInstance } from 'fastify';

Expand All @@ -18,15 +19,30 @@ describe('makePohttpServer', () => {
test('No logger should be passed by default', async () => {
await makePohttpServer();

expect(makeFastify).toHaveBeenCalledWith(expect.anything(), undefined);
expect(makeFastify).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({ logger: undefined }),
);
});

test('Any explicit logger should be honored', async () => {
const logger = pino();

await makePohttpServer(logger);

expect(makeFastify).toHaveBeenCalledWith(expect.anything(), logger);
expect(makeFastify).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({ logger }),
);
});

test('Request body should not be bigger than biggest RAMF message', async () => {
await makePohttpServer();

expect(makeFastify).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({ bodyLimit: MAX_RAMF_MESSAGE_LENGTH }),
);
});

test('Server instance should be returned', async () => {
Expand Down
5 changes: 3 additions & 2 deletions src/server/server.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { MAX_RAMF_MESSAGE_LENGTH } from '@relaycorp/relaynet-core';
import type { FastifyInstance, FastifyPluginCallback, RouteOptions } from 'fastify';
import type { BaseLogger } from 'pino';

Expand All @@ -16,6 +17,6 @@ export async function makePohttpServerPlugin(server: FastifyInstance): Promise<v
await Promise.all(rootRoutes.map((route) => server.register(route)));
}

export async function makePohttpServer(customLogger?: BaseLogger): Promise<FastifyInstance> {
return makeFastify(makePohttpServerPlugin, customLogger);
export async function makePohttpServer(logger?: BaseLogger): Promise<FastifyInstance> {
return makeFastify(makePohttpServerPlugin, { logger, bodyLimit: MAX_RAMF_MESSAGE_LENGTH });
}
2 changes: 1 addition & 1 deletion src/utilities/fastify/plugins/healthCheck.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import registerHealthCheck from './healthCheck.js';
describe('healthcheck routes', () => {
const getTestServerFixture = makeTestServer(
async (logger?: BaseLogger) =>
makeFastify(registerHealthCheck as FastifyPluginCallback, logger),
makeFastify(registerHealthCheck as FastifyPluginCallback, { logger }),
REQUIRED_ENV_VARS,
);

Expand Down
16 changes: 15 additions & 1 deletion src/utilities/fastify/server.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe('makeFastify', () => {
test('Any explicit logger should be honored', async () => {
const logger = pino();

await makeFastify(mockPlugin, logger);
await makeFastify(mockPlugin, { logger });

expect(fastify).toHaveBeenCalledWith(expect.objectContaining({ logger }));
expect(mockErrorHandler).toHaveBeenCalledWith(logger);
Expand Down Expand Up @@ -102,6 +102,20 @@ describe('makeFastify', () => {
expect(fastifyCallArguments).toHaveProperty('trustProxy', true);
});

test('Request body should not be set by default', async () => {
await makeFastify(mockPlugin);

expect(fastify).toHaveBeenCalledWith(expect.objectContaining({ bodyLimit: undefined }));
});

test('Request body should not specified if set', async () => {
const bodyLimit = 100;

await makeFastify(mockPlugin, { bodyLimit });

expect(fastify).toHaveBeenCalledWith(expect.objectContaining({ bodyLimit }));
});

test('fastify-graceful-shutdown plugin should be registered', async () => {
await makeFastify(mockPlugin);

Expand Down
15 changes: 11 additions & 4 deletions src/utilities/fastify/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,23 +32,30 @@ export const HTTP_METHODS: readonly HTTPMethods[] = [
'OPTIONS',
];

export interface FastifyOptions {
readonly logger: BaseLogger;
readonly bodyLimit: number;
}

export async function makeFastify(
appPlugin: FastifyPluginAsync | FastifyPluginCallback,
customLogger?: BaseLogger,
options: Partial<FastifyOptions> = {},
) {
const logger = customLogger ?? makeLogger();
const logger = options.logger ?? makeLogger();
configureErrorHandling(logger);

const server = fastify({
logger,

bodyLimit: options.bodyLimit,

trustProxy: true,

requestIdHeader: env
.get('REQUEST_ID_HEADER')
.default(DEFAULT_REQUEST_ID_HEADER)
.asString()
.toLowerCase(),

trustProxy: true,
});
await server.register(fastifyGracefulShutdown, { resetHandlersOnInit: true });

Expand Down
1 change: 1 addition & 0 deletions src/utilities/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export const HTTP_STATUS_CODES = {
FORBIDDEN: 403,
NOT_FOUND: 404,
METHOD_NOT_ALLOWED: 405,
CONTENT_TOO_LARGE: 413,
UNSUPPORTED_MEDIA_TYPE: 415,
INTERNAL_SERVER_ERROR: 500,
BAD_GATEWAY: 502,
Expand Down

0 comments on commit 96e6854

Please sign in to comment.