Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

Commit

Permalink
feat(ssrServer): make max POST request payload size configurable (#721)
Browse files Browse the repository at this point in the history
Co-authored-by: Matthew Mallimo <matthew.c.mallimo@aexp.com>
  • Loading branch information
10xLaCroixDrinker and Matthew-Mallimo authored May 2, 2022
1 parent 025c9ec commit a1abb49
Show file tree
Hide file tree
Showing 8 changed files with 155 additions and 29 deletions.
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`runTime HTTP_ONE_APP_DEV_CDN_PORT normalizes numeric input 1`] = `"env var HTTP_ONE_APP_DEV_CDN_PORT needs to be a valid integer, given \\"0002345a\\""`;

exports[`runTime HTTP_ONE_APP_DEV_PROXY_SERVER_PORT normalizes numeric input: env var HTTP_ONE_APP_DEV_PROXY_SERVER_PORT needs to be a valid integer, given "r00t" 1`] = `"env var HTTP_ONE_APP_DEV_PROXY_SERVER_PORT needs to be a valid integer, given \\"r00t\\""`;

exports[`runTime NODE_ENV only allows for values to be set to either development or production 1`] = `
Array [
"development",
"production",
]
`;

exports[`runTime ONE_CLIENT_ROOT_MODULE_NAME validates that environment value is defined 1`] = `"The \`ONE_CLIENT_ROOT_MODULE_NAME\` environment variable must be defined."`;
96 changes: 82 additions & 14 deletions __tests__/server/config/env/runTime.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,11 @@ describe('runTime', () => {
'ONE_CLIENT_CDN_URL',
'ONE_CLIENT_LOCALE_FILENAME',
'ONE_CLIENT_ROOT_MODULE_NAME',
]
.forEach((name) => { origEnvVarVals[name] = process.env[name]; });
'ONE_ENABLE_POST_TO_MODULE_ROUTES',
'ONE_MAX_POST_REQUEST_PAYLOAD',
].forEach((name) => {
origEnvVarVals[name] = process.env[name];
});

function getEnvVarConfig(envVarName) {
const runTime = require('../../../../src/server/config/env/runTime').default;
Expand All @@ -70,6 +73,7 @@ describe('runTime', () => {
resetEnvVar('NODE_ENV');
resetEnvVar('ONE_DANGEROUSLY_DISABLE_CSP', 'false');
resetEnvVar('HTTP_ONE_APP_DEV_CDN_PORT');
resetEnvVar('ONE_ENABLE_POST_TO_MODULE_ROUTES');
jest.resetModules();
jest.resetAllMocks();
});
Expand All @@ -78,9 +82,7 @@ describe('runTime', () => {
// eslint-disable-next-line no-console
console.info = origConsoleInfo;
console.warn = origConsoleWarn;
Object
.keys(origEnvVarVals)
.forEach((name) => resetEnvVar(name, origEnvVarVals[name]));
Object.keys(origEnvVarVals).forEach((name) => resetEnvVar(name, origEnvVarVals[name]));
jest.resetAllMocks();
});

Expand Down Expand Up @@ -131,13 +133,17 @@ describe('runTime', () => {
const disableCspEnv = getEnvVarConfig('ONE_DANGEROUSLY_DISABLE_CSP');

it('throws error if ONE_DANGEROUSLY_DISABLE_CSP is set to true and NODE_ENV is not development', () => {
expect(() => disableCspEnv.validate('true')).toThrowError('If you are trying to bypass CSP requirement, NODE_ENV must also be set to development.');
expect(() => disableCspEnv.validate('true')).toThrowError(
'If you are trying to bypass CSP requirement, NODE_ENV must also be set to development.'
);
});

it('warns console if both ONE_DANGEROUSLY_DISABLE_CSP and NODE_ENV are set properly', () => {
process.env.NODE_ENV = 'development';
disableCspEnv.validate('true');
expect(console.warn).toHaveBeenCalledWith('ONE_DANGEROUSLY_DISABLE_CSP is true and NODE_ENV is set to development. Content-Security-Policy header will not be set.');
expect(console.warn).toHaveBeenCalledWith(
'ONE_DANGEROUSLY_DISABLE_CSP is true and NODE_ENV is set to development. Content-Security-Policy header will not be set.'
);
});

it('does not warn or throw if ONE_DANGEROUSLY_DISABLE_CSP is set to false', () => {
Expand Down Expand Up @@ -190,7 +196,9 @@ describe('runTime', () => {
'env var HTTP_ONE_APP_DEV_CDN_PORT needs to be a valid integer, given "r00t"'
);
expect(devCdnPort.normalize('0002345')).toEqual(2345);
expect(() => devCdnPort.normalize('0002345a')).toThrowErrorMatchingSnapshot();
expect(() => devCdnPort.normalize('0002345a')).toThrowErrorMatchingInlineSnapshot(
'"env var HTTP_ONE_APP_DEV_CDN_PORT needs to be a valid integer, given \\"0002345a\\""'
);
});

it('does not normalize if no value is given', () => {
Expand All @@ -215,8 +223,8 @@ describe('runTime', () => {

it('normalizes numeric input', () => {
expect(devProxyPort.normalize('1337')).toEqual(1337);
expect(() => devProxyPort.normalize('r00t')).toThrowErrorMatchingSnapshot(
'env var HTTP_ONE_APP_DEV_PROXY_SERVER_PORT needs to be a valid integer, given "r00t"'
expect(() => devProxyPort.normalize('r00t')).toThrowErrorMatchingInlineSnapshot(
'"env var HTTP_ONE_APP_DEV_PROXY_SERVER_PORT needs to be a valid integer, given \\"r00t\\""'
);
expect(devProxyPort.normalize('0002345')).toEqual(2345);
expect(() => devProxyPort.normalize('0002345a')).toThrow();
Expand Down Expand Up @@ -263,7 +271,9 @@ describe('runTime', () => {
process.env.NODE_ENV = 'development';
process.env.HTTP_ONE_APP_DEV_CDN_PORT = 3001;
expect(holocronModuleMapPath.defaultValue()).toBeDefined();
expect(holocronModuleMapPath.defaultValue()).toBe('http://localhost:3001/static/module-map.json');
expect(holocronModuleMapPath.defaultValue()).toBe(
'http://localhost:3001/static/module-map.json'
);
});

it('has no default value for production', () => {
Expand Down Expand Up @@ -293,13 +303,18 @@ describe('runTime', () => {
});

describe('HOLOCRON_SERVER_MAX_SIM_MODULES_FETCH', () => {
const holocronServerMaxSimModulesFetch = getEnvVarConfig('HOLOCRON_SERVER_MAX_SIM_MODULES_FETCH');
const holocronServerMaxSimModulesFetch = getEnvVarConfig(
'HOLOCRON_SERVER_MAX_SIM_MODULES_FETCH'
);

it('does not have a default value', () => {
expect(holocronServerMaxSimModulesFetch.defaultValue).toBe(undefined);
});

it('validates the value as a positive integer', positiveInteger(holocronServerMaxSimModulesFetch));
it(
'validates the value as a positive integer',
positiveInteger(holocronServerMaxSimModulesFetch)
);
});

describe('ONE_CLIENT_REPORTING_URL', () => {
Expand Down Expand Up @@ -390,7 +405,9 @@ describe('runTime', () => {
const clientRootModuleName = getEnvVarConfig('ONE_CLIENT_ROOT_MODULE_NAME');

it('validates that environment value is defined', () => {
expect(() => clientRootModuleName.validate()).toThrowErrorMatchingSnapshot();
expect(() => clientRootModuleName.validate()).toThrowErrorMatchingInlineSnapshot(
'"The `ONE_CLIENT_ROOT_MODULE_NAME` environment variable must be defined."'
);
expect(() => clientRootModuleName.validate('frank-lloyd-root')).not.toThrow();
});

Expand Down Expand Up @@ -432,4 +449,55 @@ describe('runTime', () => {
expect(oneServiceWorkerFeatureFlag.normalize('true')).toEqual(true);
});
});

describe('ONE_ENABLE_POST_TO_MODULE_ROUTES', () => {
const enablePostToModuleRoutes = getEnvVarConfig('ONE_ENABLE_POST_TO_MODULE_ROUTES');

it('should have a default value of "false"', () => {
expect(enablePostToModuleRoutes.defaultValue).toBe('false');
});

it('should normalize the value to lower case', () => {
expect(enablePostToModuleRoutes.normalize('Value')).toBe('value');
expect(enablePostToModuleRoutes.normalize('VALUE')).toBe('value');
});

it('should pass validation when value is "true" or "false"', () => {
expect(() => enablePostToModuleRoutes.validate('true')).not.toThrow();
expect(() => enablePostToModuleRoutes.validate('false')).not.toThrow();
});

it('should fail validation when value is not "true" or "false"', () => {
expect(() => enablePostToModuleRoutes.validate('bad value')
).toThrowErrorMatchingInlineSnapshot(
'"Expected \\"bad value\\" to be \\"true\\" or \\"false\\""'
);
});
});

describe('ONE_MAX_POST_REQUEST_PAYLOAD', () => {
const postRequestMaxPayload = getEnvVarConfig('ONE_MAX_POST_REQUEST_PAYLOAD');

it('should have a default value of "15kb"', () => {
expect(postRequestMaxPayload.defaultValue).toBe('15kb');
});

it('should fail validation when input is not parseable by bytes util', () => {
process.env.ONE_ENABLE_POST_TO_MODULE_ROUTES = true;
expect(() => postRequestMaxPayload.validate('bad value')).toThrowErrorMatchingInlineSnapshot(
'"Expected \\"bad value\\" to be parseable by bytes utility https://www.npmjs.com/package/bytes"'
);
});

it('should pass validation when input is parseable by bytes util', () => {
process.env.ONE_ENABLE_POST_TO_MODULE_ROUTES = true;
expect(() => postRequestMaxPayload.validate('20kb')).not.toThrow();
});

it('should fail validation when POSTing is not enabled', () => {
expect(() => postRequestMaxPayload.validate('20kb')).toThrowErrorMatchingInlineSnapshot(
'"ONE_ENABLE_POST_TO_MODULE_ROUTES must be \\"true\\" to configure max POST payload."'
);
});
});
});
9 changes: 6 additions & 3 deletions __tests__/server/ssrServer.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ describe('ssrServer', () => {
jest.clearAllMocks();
process.env.NODE_ENV = 'development';
delete process.env.ONE_ENABLE_POST_TO_MODULE_ROUTES;
delete process.env.ONE_MAX_POST_REQUEST_PAYLOAD;
});

describe('middleware order', () => {
Expand Down Expand Up @@ -288,7 +289,7 @@ describe('ssrServer', () => {

describe('enablePostToModuleRoutes', () => {
beforeEach(() => {
process.env.ONE_ENABLE_POST_TO_MODULE_ROUTES = 'yes';
process.env.ONE_ENABLE_POST_TO_MODULE_ROUTES = 'true';
});

it('should check the state for a status code for all post calls', async () => {
Expand Down Expand Up @@ -331,10 +332,11 @@ describe('ssrServer', () => {
});

it('should configure json parsing with a maximum limit for render post calls', async () => {
process.env.ONE_MAX_POST_REQUEST_PAYLOAD = '100kb';
await request(loadServer())
.post('/route');
expect(json).toBeCalled();
expect(json.mock.calls[2][0]).toHaveProperty('limit', '15kb');
expect(json.mock.calls[2][0]).toHaveProperty('limit', '100kb');
});
it('should configure urlencoded parsing with a maximum limit for render post pre-flight options calls', async () => {
await request(loadServer())
Expand All @@ -344,10 +346,11 @@ describe('ssrServer', () => {
});

it('should configure json urlencoded with a maximum limit for render post calls', async () => {
process.env.ONE_MAX_POST_REQUEST_PAYLOAD = '25kb';
await request(loadServer())
.post('/route');
expect(urlencoded).toBeCalled();
expect(json.mock.calls[2][0]).toHaveProperty('limit', '15kb');
expect(json.mock.calls[2][0]).toHaveProperty('limit', '25kb');
});

describe('cors for render post calls', () => {
Expand Down
33 changes: 32 additions & 1 deletion docs/api/server/Environment-Variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ One App can be configured via Environment Variables:
* [`HOLOCRON_SERVER_MAX_MODULES_RETRY`](#holocron_server_max_modules_retry)
* [`HOLOCRON_SERVER_MAX_SIM_MODULES_FETCH`](#holocron_server_max_sim_modules_fetch)
* [`ONE_ENABLE_POST_TO_MODULE_ROUTES`](#one_enable_post_to_module_routes)
* [`ONE_MAX_POST_REQUEST_PAYLOAD`](#one_max_post_request_payload)
* [`ONE_MAP_POLLING_MAX`](#one_map_polling_max)
* [`ONE_MAP_POLLING_MIN`](#one_map_polling_min)
* [`ONE_REFERRER_POLICY_OVERRIDE`](#one_referrer_policy_override)
Expand Down Expand Up @@ -71,6 +72,7 @@ One App can be configured via Environment Variables:
* [`ONE_ENABLE_POST_TO_MODULE_ROUTES`](#one_enable_post_to_module_routes)
* [`ONE_MAP_POLLING_MAX`](#one_map_polling_max)
* [`ONE_MAP_POLLING_MIN`](#one_map_polling_min)
* [`ONE_MAX_POST_REQUEST_PAYLOAD`](#one_max_post_request_payload)
* [`ONE_REFERRER_POLICY_OVERRIDE`](#one_referrer_policy_override)
* [`ONE_SERVICE_WORKER`](#one_service_worker)
</details>
Expand Down Expand Up @@ -589,18 +591,46 @@ ONE_DANGEROUSLY_DISABLE_CSP=false
* ✅ Production
* ✅ Development
Set if One App should respond to POST requests.
Set to true if One App should respond to POST requests.
**Shape**
```bash
ONE_ENABLE_POST_TO_MODULE_ROUTES=Boolean
```
**Example**
```bash
ONE_ENABLE_POST_TO_MODULE_ROUTES=true
```
**Default Value**
```bash
ONE_ENABLE_POST_TO_MODULE_ROUTES=false
```
## `ONE_MAX_POST_REQUEST_PAYLOAD`
**Runs In**
* ✅ Production
* ✅ Development
Maximum payload allowed in POST requests. `ONE_ENABLE_POST_TO_MODULE_ROUTES` must be true to configure this.
**Shape**
```bash
ONE_MAX_POST_REQUEST_PAYLOAD=String
```
**Example**
```bash
ONE_MAX_POST_REQUEST_PAYLOAD=100kb
```
**Default Value**
```bash
ONE_MAX_POST_REQUEST_PAYLOAD=15kb
```
## `ONE_MAP_POLLING_MAX`
**Runs In**
Expand Down Expand Up @@ -684,6 +714,7 @@ ONE_SERVICE_WORKER=true
ONE_SERVICE_WORKER=false
```
**📘 More Information**
* Useful NodeJS Env Variables: [Node CLI Docs](https://nodejs.org/api/cli.html#cli_node_extra_ca_certs_file)
* [Development Tools Documentation](./Development-Tools.md)
Expand Down
1 change: 1 addition & 0 deletions package-lock.json

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

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"contributors": [
"Andres Escobar <Andres.Escobar@aexp.com> (https://github.com/anescobar1991)",
"James Singleton <James.Singleton1@aexp.com> (https://github.com/JamesSingleton)",
"Jimmy King <Jimmy.King@aexp.com> (https://github.com/10xLaCroixDrinker)",
"Jamie King <Jamie.King@aexp.com> (https://github.com/10xLaCroixDrinker)",
"Jonathan Adshead <Jonathan.Adshead@aexp.com> (https://github.com/JAdshead)",
"Michael Tobia <Michael.M.Tobia@aexp.com> (https://github.com/Francois-Esquire)",
"Michael Tomcal <Michael.A.Tomcal@aexp.com> (https://github.com/mtomcal)",
Expand Down Expand Up @@ -87,6 +87,7 @@
"@americanexpress/vitruvius": "^2.0.2",
"abort-controller": "^3.0.0",
"body-parser": "^1.19.0",
"bytes": "^3.1.2",
"chalk": "^4.1.2",
"compression": "^1.7.4",
"cookie-parser": "^1.4.5",
Expand Down
30 changes: 29 additions & 1 deletion src/server/config/env/runTime.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const { preprocessEnvVar } = require('@americanexpress/env-config-utils');
const isFetchableUrlInNode = require('@americanexpress/env-config-utils/isFetchableUrlInNode');
const isFetchableUrlInBrowser = require('@americanexpress/env-config-utils/isFetchableUrlInBrowser');
const { argv } = require('yargs');
const bytes = require('bytes');

const isPositiveIntegerIfDefined = (input) => {
if (input === undefined) {
Expand Down Expand Up @@ -197,7 +198,7 @@ const runTime = [
];

if (!approvedPolicies.includes(value)) {
throw new Error(`${value} in not an approved policy. Please use: ${approvedPolicies.join(',')}.`);
throw new Error(`"${value}" is not an approved policy. Please use: ${approvedPolicies.join(',')}.`);
}
},
},
Expand All @@ -208,6 +209,33 @@ const runTime = [
normalize: (value) => value === 'true',
defaultValue: () => false,
},
// Enable POSTing to module routes
{
name: 'ONE_ENABLE_POST_TO_MODULE_ROUTES',
defaultValue: 'false',
normalize: (input) => input.toLowerCase(),
validate: (input) => {
if (input !== 'true' && input !== 'false') {
throw new Error(`Expected "${input}" to be "true" or "false"`);
}
},
},
// Customize max payload for POST requests
{
name: 'ONE_MAX_POST_REQUEST_PAYLOAD',
defaultValue: '15kb',
validate: (input) => {
if (process.env.ONE_ENABLE_POST_TO_MODULE_ROUTES !== 'true') {
throw new Error('ONE_ENABLE_POST_TO_MODULE_ROUTES must be "true" to configure max POST payload.');
}

const parsed = bytes.parse(input);

if (parsed === null) {
throw new Error(`Expected "${input}" to be parseable by bytes utility https://www.npmjs.com/package/bytes`);
}
},
},
];
runTime.forEach(preprocessEnvVar);
export { ip };
Expand Down
6 changes: 3 additions & 3 deletions src/server/ssrServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ export function createApp({ enablePostToModuleRoutes = false } = {}) {
app.post(
'*',
addSecurityHeaders,
json({ limit: '15kb' }),
urlencoded({ limit: '15kb' }),
json({ limit: process.env.ONE_MAX_POST_REQUEST_PAYLOAD }),
urlencoded({ limit: process.env.ONE_MAX_POST_REQUEST_PAYLOAD }),
addFrameOptionsHeader,
createRequestStore(oneApp, { useBodyForBuildingTheInitialState: true }),
createRequestHtmlFragment(oneApp),
Expand All @@ -123,5 +123,5 @@ export function createApp({ enablePostToModuleRoutes = false } = {}) {
}

export default createApp({
enablePostToModuleRoutes: !!process.env.ONE_ENABLE_POST_TO_MODULE_ROUTES,
enablePostToModuleRoutes: process.env.ONE_ENABLE_POST_TO_MODULE_ROUTES === 'true',
});

0 comments on commit a1abb49

Please sign in to comment.