diff --git a/README.md b/README.md index a1b97910c4..392242a9f6 100644 --- a/README.md +++ b/README.md @@ -262,6 +262,8 @@ Grist can be configured in many ways. Here are the main environment variables it | APP_STATIC_URL | url prefix for static resources | | APP_STATIC_INCLUDE_CUSTOM_CSS | set to "true" to include custom.css (from APP_STATIC_URL) in static pages | | APP_UNTRUSTED_URL | URL at which to serve/expect plugin content. | +| GRIST_ACTION_HISTORY_MAX_ROWS | Maximum number of rows allowed in ActionHistory before pruning (up to a 1.25 grace factor). Defaults to 1000. ⚠️ A too low value may make the "[Work on a copy](https://support.getgrist.com/newsletters/2021-06/#work-on-a-copy)" feature [malfunction](https://github.com/gristlabs/grist-core/issues/1121#issuecomment-2248112023) | +| GRIST_ACTION_HISTORY_MAX_BYTES | Maximum number of rows allowed in ActionHistory before pruning (up to a 1.25 grace factor). Defaults to 1Gb. ⚠️ A too low value may make the "[Work on a copy](https://support.getgrist.com/newsletters/2021-06/#work-on-a-copy)" feature [malfunction](https://github.com/gristlabs/grist-core/issues/1121#issuecomment-2248112023) | | GRIST_ADAPT_DOMAIN | set to "true" to support multiple base domains (careful, host header should be trustworthy) | | GRIST_APP_ROOT | directory containing Grist sandbox and assets (specifically the sandbox and static subdirectories). | | GRIST_BACKUP_DELAY_SECS | wait this long after a doc change before making a backup | diff --git a/app/server/lib/ActionHistoryImpl.ts b/app/server/lib/ActionHistoryImpl.ts index 3930d864ec..b1e2c83c02 100644 --- a/app/server/lib/ActionHistoryImpl.ts +++ b/app/server/lib/ActionHistoryImpl.ts @@ -12,14 +12,28 @@ import mapValues = require('lodash/mapValues'); import {ActionGroupOptions, ActionHistory, ActionHistoryUndoInfo, asActionGroup, asMinimalActionGroup} from './ActionHistory'; import {ISQLiteDB, ResultRow} from './SQLiteDB'; +import { appSettings } from './AppSettings'; + +const section = appSettings.section('history').section('action'); // History will from time to time be pruned back to within these limits // on rows and the maximum total number of bytes in the "body" column. // Pruning is done when the history has grown above these limits, to // the specified factor. -const ACTION_HISTORY_MAX_ROWS = 1000; -const ACTION_HISTORY_MAX_BYTES = 1000 * 1000 * 1000; // 1 GB. -const ACTION_HISTORY_GRACE_FACTOR = 1.25; // allow growth to 1250 rows / 1.25 GB. +const ACTION_HISTORY_MAX_ROWS = section.flag('maxRows').requireInt({ + envVar: 'GRIST_ACTION_HISTORY_MAX_ROWS', + defaultValue: 1000, + + minValue: 1, +}); + +const ACTION_HISTORY_MAX_BYTES = section.flag('maxBytes').requireInt({ + envVar: 'GRIST_ACTION_HISTORY_MAX_BYTES', + defaultValue: 1e9, // 1 GB. + minValue: 1, // 1 B. +}); + +const ACTION_HISTORY_GRACE_FACTOR = 1.25; // allow growth to 1.25 times the above limits. const ACTION_HISTORY_CHECK_PERIOD = 10; // number of actions between size checks. /** diff --git a/app/server/lib/AppSettings.ts b/app/server/lib/AppSettings.ts index 1bf9ccba53..42908f7bf0 100644 --- a/app/server/lib/AppSettings.ts +++ b/app/server/lib/AppSettings.ts @@ -99,7 +99,7 @@ export class AppSettings { /** * As for readInt() but fail if nothing was found. */ - public requireInt(query: AppSettingQuery): number { + public requireInt(query: AppSettingQueryInt): number { const result = this.readInt(query); if (result === undefined) { throw new Error(`missing environment variable: ${query.envVar}`); @@ -122,9 +122,19 @@ export class AppSettings { * As for read() but type (and store, and report) the result as * an integer (well, a number). */ - public readInt(query: AppSettingQuery): number|undefined { + public readInt(query: AppSettingQueryInt): number|undefined { this.readString(query); const result = this.getAsInt(); + + if (result !== undefined) { + if (query.minValue !== undefined && result < query.minValue) { + throw new Error(`value ${result} is less than minimum ${query.minValue}`); + } + if (query.maxValue !== undefined && result > query.maxValue) { + throw new Error(`value ${result} is greater than maximum ${query.maxValue}`); + } + } + this._value = result; return result; } @@ -213,11 +223,39 @@ export const appSettings = new AppSettings('grist'); * environment variables and default values. */ export interface AppSettingQuery { - envVar: string|string[]; // environment variable(s) to check. - preferredEnvVar?: string; // "Canonical" environment variable to suggest. - // Should be in envVar (though this is not checked). - defaultValue?: JSONValue; // value to use if variable(s) unavailable. - censor?: boolean; // should the value of the setting be obscured when printed. + /** + * Environment variable(s) to check. + */ + envVar: string|string[]; + /** + * "Canonical" environment variable to suggest. Should be in envVar (though this is not checked). + */ + preferredEnvVar?: string; + /** + * Value to use if the variable(s) is/are unavailable. + */ + defaultValue?: JSONValue; + /** + * When set to true, the value is obscured when printed. + */ + censor?: boolean; +} + +export interface AppSettingQueryInt extends AppSettingQuery { + /** + * Value to use if variable(s) unavailable. + */ + defaultValue?: number; + /** + * Minimum value allowed. Raises an error if the value is lower than this. + * If the value is undefined, the setting is not checked. + */ + minValue?: number; + /** + * Maximum value allowed. Raises an error if the value is greater than this. + * If the value is undefined, the setting is not checked. + */ + maxValue?: number; } /** diff --git a/app/server/lib/OIDCConfig.ts b/app/server/lib/OIDCConfig.ts index 2343f468a1..9e500d6c4f 100644 --- a/app/server/lib/OIDCConfig.ts +++ b/app/server/lib/OIDCConfig.ts @@ -141,6 +141,7 @@ export class OIDCConfig { }); const httpTimeout = section.flag('httpTimeout').readInt({ envVar: 'GRIST_OIDC_SP_HTTP_TIMEOUT', + minValue: 0, // 0 means no timeout }); this._namePropertyKey = section.flag('namePropertyKey').readString({ envVar: 'GRIST_OIDC_SP_PROFILE_NAME_ATTR', diff --git a/test/server/lib/AppSettings.ts b/test/server/lib/AppSettings.ts new file mode 100644 index 0000000000..77ab06d7f2 --- /dev/null +++ b/test/server/lib/AppSettings.ts @@ -0,0 +1,96 @@ +import { AppSettings } from 'app/server/lib/AppSettings'; +import { EnvironmentSnapshot } from '../testUtils'; + +import { assert } from 'chai'; + +describe('AppSettings', () => { + let appSettings: AppSettings; + let env: EnvironmentSnapshot; + beforeEach(() => { + appSettings = new AppSettings('test'); + env = new EnvironmentSnapshot(); + }); + + afterEach(() => { + env.restore(); + }); + + describe('for integers', () => { + function testIntMethod(method: 'readInt' | 'requireInt') { + it('should throw an error if the value is less than the minimum', () => { + process.env.TEST = '4'; + assert.throws(() => { + appSettings[method]({ envVar: 'TEST', minValue: 5 }); + }, 'value 4 is less than minimum 5'); + }); + + it('should throw an error if the value is greater than the maximum', () => { + process.env.TEST = '6'; + assert.throws(() => { + appSettings[method]({ envVar: 'TEST', maxValue: 5 }); + }, 'value 6 is greater than maximum 5'); + }); + + it('should throw if the value is NaN', () => { + process.env.TEST = 'not a number'; + assert.throws(() => appSettings[method]({ envVar: 'TEST' }), 'not a number does not look like a number'); + }); + + it('should throw if the default value is not finite', () => { + assert.throws( + () => appSettings[method]({ envVar: 'TEST', defaultValue: Infinity }), + 'Infinity does not look like a number' + ); + }); + + it('should throw if the default value is not within the range', () => { + assert.throws( + () => appSettings[method]({ + envVar: 'TEST', + defaultValue: 6, + minValue: 7, + maxValue: 9, + }), + 'value 6 is less than minimum 7' + ); + }); + + it('should return the default value if it is within the range', () => { + const result = appSettings[method]({ + envVar: 'TEST', + defaultValue: 5, + minValue: 5, + maxValue: 12 + }); + assert.strictEqual(result, 5); + }); + + it('should return the value if it is within the range', () => { + process.env.TEST = '5'; + assert.strictEqual(appSettings[method]({ envVar: 'TEST', minValue: 5 }), 5); + }); + + it('should return the integer value of a float', () => { + process.env.TEST = '5.9'; + assert.strictEqual(appSettings[method]({ envVar: 'TEST' }), 5); + }); + } + + describe('readInt()', () => { + testIntMethod('readInt'); + + it('should return undefined when no value nor default value is passed', () => { + const result = appSettings.readInt({ envVar: 'TEST', maxValue: 5 }); + assert.isUndefined(result); + }); + }); + + describe('requireInt()', () => { + testIntMethod('requireInt'); + + it('should throw if env variable is not set and no default value is passed', () => { + assert.throws(() => appSettings.requireInt({ envVar: 'TEST' }), 'missing environment variable: TEST'); + }); + }); + }); +});