Skip to content

Commit

Permalink
Use AppSettings and introduce minValue and maxValue
Browse files Browse the repository at this point in the history 
AppSettings seems more future-proof and already raises error when passed
value is not a number. We also introduce minValue and maxValue so we can
constraint the configured value to be within a certain range and avoid
erratic behavior when the value has not been expected.
  • Loading branch information
@fflorent
fflorent committed Oct 15, 2024
1 parent f0cae5f commit 07adb83
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 11 deletions.
4 changes: 2 additions & 2 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,6 @@ Grist can be configured in many ways. Here are the main environment variables it

| Variable | Purpose |
|------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| ACTION_HISTORY_MAX_ROWS | Maximum number of rows allowed in ActionHistory before pruning (up to a 1.25 grace factor). Defaults to 1000. |
| ACTION_HISTORY_MAX_BYTES | Maximum number of rows allowed in ActionHistory before pruning (up to a 1.25 grace factor). Defaults to 1.25Gb. |
| ALLOWED_WEBHOOK_DOMAINS | comma-separated list of permitted domains to use in webhooks (e.g. webhook.site,zapier.com). You can set this to `*` to allow all domains, but if doing so, we recommend using a carefully locked-down proxy (see `GRIST_HTTPS_PROXY`) if you do not entirely trust users. Otherwise services on your internal network may become vulnerable to manipulation. |
| APP_DOC_URL | doc worker url, set when starting an individual doc worker (other servers will find doc worker urls via redis) |
| APP_DOC_INTERNAL_URL | like `APP_DOC_URL` but used by the home server to reach the server using an internal domain name resolution (like in a docker environment). It only makes sense to define this value in the doc worker. Defaults to `APP_DOC_URL`. |
Expand All @@ -264,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. |
| GRIST_ACTION_HISTORY_MAX_BYTES | Maximum number of rows allowed in ActionHistory before pruning (up to a 1.25 grace factor). Defaults to 1.25Gb. |
| 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 |
Expand Down
18 changes: 16 additions & 2 deletions app/server/lib/ActionHistoryImpl.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,27 @@ 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 = parseInt(process.env.ACTION_HISTORY_MAX_ROWS, 10) || 1000;
const ACTION_HISTORY_MAX_BYTES = parseInt(process.env.ACTION_HISTORY_MAX_BYTES, 10) || 1000 * 1000 * 1000; // 1 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: 1_000_0000_0000, // 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.

Expand Down
52 changes: 45 additions & 7 deletions app/server/lib/AppSettings.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}

/**
Expand Down
1 change: 1 addition & 0 deletions app/server/lib/OIDCConfig.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit 07adb83

Please sign in to comment.