Skip to content

Commit

Permalink
feat(config): property type and value checking
Browse files Browse the repository at this point in the history
This adds assertion logic to the `Config` class to validate that any
properties specified are of the expected type and, for certain
properties, that the value is valid. This may prevent crashes or other
unexpected behavior in xud due to invalid configuration, and will also
log more instructive errors when a specified property is invalid.

Closes #1222.
  • Loading branch information
sangaman committed Feb 25, 2020
1 parent d376e4c commit 7c8590f
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 6 deletions.
47 changes: 42 additions & 5 deletions lib/Config.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,47 @@
import assert from 'assert';
import { promises as fs } from 'fs';
import os from 'os';
import path from 'path';
import toml from 'toml';
import { deepMerge } from './utils/utils';
import { promises as fs } from 'fs';
import { XuNetwork } from './constants/enums';
import { LndClientConfig } from './lndclient/types';
import { RaidenClientConfig } from './raidenclient/types';
import { Level } from './Logger';
import { XuNetwork } from './constants/enums';
import { PoolConfig } from './p2p/types';
import { OrderBookThresholds } from './orderbook/types';
import { PoolConfig } from './p2p/types';
import { RaidenClientConfig } from './raidenclient/types';
import { deepMerge } from './utils/utils';

const propAssertions = {
port: (val: number) => assert(val >= 0 && val <= 65535, 'port must be between 0 and 65535'),
cltvdelta: (val: number) => assert(val > 0, 'cltvdelta must be a positive number'),
discoverminutes: (val: number) => assert(val > 0, 'discoverminutes must be a positive number'),
minQuantity: (val: number) => assert(val >= 0, 'minQuantity must be 0 or a positive number'),
};

function validateConfig(propVal: any, defaultVal: any, propKey?: string, prefix?: string) {
const actualType = typeof propVal;
const expectedType = typeof defaultVal;
if (actualType === 'undefined') {
return; // this is an unspecified property that will use the default value
}
if (expectedType === 'undefined') {
return; // this is a superfluous property that we ignore for now
}
assert.equal(actualType, expectedType, `${prefix || ''}${propKey} is type ${actualType} but should be ${expectedType}`);

if (actualType === 'object') {
// if this is an object, we recurse
for (const nestedPropKey in propVal) {
const nestedPrefix = propKey ? `${prefix || ''}${propKey}.` : undefined;
validateConfig(propVal[nestedPropKey], defaultVal[nestedPropKey], nestedPropKey, nestedPrefix);
}
} else {
if (propKey && propKey in propAssertions) {
// shortcoming in typescript 3.6.4 `in` keyword type guard requires manual cast to any below
(propAssertions as any)[propKey](propVal);
}
}
}

class Config {
public p2p: PoolConfig;
Expand Down Expand Up @@ -176,6 +209,8 @@ class Config {
const configProps = await Config.readConfigProps(configPath);

if (configProps) {
validateConfig(configProps, this);

// set the network and xudir props up front because they influence default config values
if (configProps.network && (!args || !args.network)) {
this.network = configProps.network;
Expand Down Expand Up @@ -208,6 +243,8 @@ class Config {
}

if (args) {
validateConfig(args, this);

// override our config file with command line arguments
deepMerge(this, args);
}
Expand Down
14 changes: 13 additions & 1 deletion lib/Xud.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import Service from './service/Service';
import SwapClientManager from './swaps/SwapClientManager';
import Swaps from './swaps/Swaps';
import { UnitConverter } from './utils/UnitConverter';
import { AssertionError } from 'assert';

const version: string = require('../package.json').version;

Expand Down Expand Up @@ -64,7 +65,18 @@ class Xud extends EventEmitter {
* @param args optional arguments to override configuration parameters.
*/
public start = async (args?: { [argName: string]: any }) => {
const configFileLoaded = await this.config.load(args);
let configFileLoaded: boolean;
try {
configFileLoaded = await this.config.load(args);
} catch (err) {
if (err instanceof AssertionError) {
console.error(err.message);
process.exit(1);
} else {
throw err;
}
}

const loggers = Logger.createLoggers(this.config.loglevel, this.config.logpath, this.config.instanceid, this.config.logdateformat);
this.logger = loggers.global;
if (configFileLoaded) {
Expand Down
22 changes: 22 additions & 0 deletions test/jest/Config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,26 @@ describe('Config', () => {
expect(config.p2p.port).toEqual(MAINNET_P2P_PORT);
expect(config.network).toEqual(XuNetwork.TestNet);
});

test('it throws an error when a property is assigned the wrong type', async () => {
await expect(config.load({ initdb: 23 })).rejects.toThrow('initdb is type number but should be boolean');
});

test('it throws an error when a nested property is assigned the wrong type', async () => {
await expect(config.load({ p2p: { listen: 'no' } })).rejects.toThrow('p2p.listen is type string but should be boolean');
});

test('it throws an error when a port property is assigned an invalid value', async () => {
await expect(config.load({ p2p: { port: 999999 } })).rejects.toThrow('port must be between 0 and 65535');
});

test('it throws an error when a cltvdelta property is assigned a negative value', async () => {
await expect(config.load({ lnd: { BTC: { cltvdelta: -1 } } })).rejects.toThrow('cltvdelta must be a positive number');
});

test('it uses the default value when a prperty is assigned an undefined value', async () => {
const defaultInitDb = config.initdb;
await config.load({ initdb: undefined });
expect(config.initdb).toEqual(defaultInitDb);
});
});

0 comments on commit 7c8590f

Please sign in to comment.