Skip to content

Commit

Permalink
Improved options validation by moving it into the updateOptions funct…
Browse files Browse the repository at this point in the history
…ion.
  • Loading branch information
PaulDalek committed Jan 23, 2025
1 parent 4366db6 commit ee8a7ea
Show file tree
Hide file tree
Showing 10 changed files with 64 additions and 93 deletions.
7 changes: 6 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ The `singleExport()`, `batchExport()`, and `startExport()` functions must be pro

Essentially, all options can be configured through `.env`, the CLI, and prompts, with one exception: the `HIGHCHARTS_ADMIN_TOKEN`, which is only available as an environment variable.

## Options Validation

By default, options validation is enabled, and it is recommended to keep it enabled to ensure that the provided options are correctly checked, validated, and parsed, allowing the exporting process to function without issues. However, it is possible to disable validation (by setting the `validation` option to **false**) if you are confident in the accuracy of the data you provide. Additionaly, when used as a Node.js module, each API function that updates global options with the provided data also offers the ability to validate the data.

## Default JSON Config

The JSON below represents the default configuration stored in the `./lib/schemas/config.js` file. If no `.env` file is found (more details on the file and environment variables below), these options will be used. The configuration is not recommended to be modified directly, as it can typically be managed through other sources.
Expand Down Expand Up @@ -766,10 +770,11 @@ This package supports both CommonJS and ES modules.

- `@returns {Object}` A copy of the global options object, or a reference to the global options object.

- `function updateOptions(newOptions, getCopy = false)`: Updates a copy of the global options object or a reference to the global options object, based on the `getCopy` flag.
- `function updateOptions(newOptions, getCopy = false, strictCheck = true)`: Updates either a copy of the global options object or a reference to the global options object, depending on the getCopy flag, using the provided newOptions, which may or may not be validated.

- `@param {Object} newOptions` - An object containing the new options to be merged into the global options.
- `@param {boolean} [getCopy=false]` - Determines whether to merge the new options into a copy of the global options object (`true`) or directly into the global options object (`false`). The default value is `false`.
- `@param {boolean} [strictCheck=true]` - Determines if stricter validation should be applied. The default value is `true`.

- `@returns {Object}` The updated options object, either the modified global options or a modified copy, based on the value of `getCopy`.

Expand Down
4 changes: 2 additions & 2 deletions dist/index.cjs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/index.esm.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/index.esm.js.map

Large diffs are not rendered by default.

13 changes: 1 addition & 12 deletions lib/chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,7 @@ See LICENSE file in root for details.

import { readFileSync, writeFileSync } from 'fs';

import {
isAllowedConfig,
updateOptions,
validateOption,
validateOptions
} from './config.js';
import { isAllowedConfig, updateOptions, validateOption } from './config.js';
import { log, logWithStack } from './logger.js';
import { getPoolStats, killPool, postWork } from './pool.js';
import { sanitize } from './sanitize.js';
Expand Down Expand Up @@ -68,9 +63,6 @@ let allowCodeExecution = false;
export async function singleExport(options) {
// Check if the export makes sense
if (options && options.export) {
// Validate single export options
options = validateOptions(options);

// Perform an export
await startExport(
{ export: options.export, customLogic: options.customLogic },
Expand Down Expand Up @@ -140,9 +132,6 @@ export async function singleExport(options) {
export async function batchExport(options) {
// Check if the export makes sense
if (options && options.export && options.export.batch) {
// Validate batch export options
options = validateOptions(options);

// An array for collecting batch exports
const batchFunctions = [];

Expand Down
44 changes: 21 additions & 23 deletions lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,9 @@ export function getOptions(getCopy = true) {
}

/**
* Updates a copy of the global options object or a reference to the global
* options object, based on the `getCopy` flag.
* Updates either a copy of the global options object or a reference
* to the global options object, depending on the getCopy flag, using
* the provided newOptions, which may or may not be validated.
*
* @function updateOptions
*
Expand All @@ -67,13 +68,20 @@ export function getOptions(getCopy = true) {
* @param {boolean} [getCopy=false] - Determines whether to merge the new
* options into a copy of the global options object (`true`) or directly into
* the global options object (`false`). The default value is `false`.
* @param {boolean} [strictCheck=true] - Determines if stricter validation
* should be applied. The default value is `true`.
*
* @returns {Object} The updated options object, either the modified global
* options or a modified copy, based on the value of `getCopy`.
*/
export function updateOptions(newOptions, getCopy = false) {
export function updateOptions(newOptions, getCopy = false, strictCheck = true) {
// Merge new options to the global options or its copy and return the result
return _mergeOptions(getOptions(getCopy), newOptions);
return _mergeOptions(
// First, get the options
getOptions(getCopy),
// Next, validate the new options
validateOptions(newOptions, strictCheck)
);
}

/**
Expand All @@ -99,33 +107,23 @@ export function setCliOptions(cliArgs) {
// Only for the CLI usage
if (cliArgs && Array.isArray(cliArgs) && cliArgs.length) {
try {
// Validate options from the custom JSON loaded via the `--loadConfig`
const configOptions = strictValidate(_loadConfigFile(cliArgs));
// Get options from the custom JSON loaded via the `--loadConfig`
const configOptions = _loadConfigFile(cliArgs);

// Update global options with the values from the `configOptions`
// Update global options with validated values from the `configOptions`
updateOptions(configOptions);
} catch (error) {
logZodIssues(
1,
error.issues,
'[validation] Custom options from the `loadConfig` option validation error'
);
log(2, '[validation] No options added from the `--loadConfig` option.');
}

try {
// Validate options from the CLI
const cliOptions = looseValidate(
_pairArgumentValue(nestedProps, cliArgs)
);
// Get options from the CLI
const cliOptions = _pairArgumentValue(nestedProps, cliArgs);

// Update global options with the values from the `cliOptions`
updateOptions(cliOptions);
// Update global options with validated values from the `cliOptions`
updateOptions(cliOptions, false, false);
} catch (error) {
logZodIssues(
1,
error.issues,
'[validation] CLI options validation error'
);
log(2, '[validation] No options added from the CLI arguments.');
}
}

Expand Down
42 changes: 18 additions & 24 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ import server from './server/server.js';
*/
export async function initExport(initOptions) {
// Init, validate and update the options object
const options = updateOptions(validateOptions(initOptions));
const options = updateOptions(initOptions);

// Set the `allowCodeExecution` per export module scope
setAllowCodeExecution(options.customLogic.allowCodeExecution);
Expand Down Expand Up @@ -155,41 +155,35 @@ export default {
logZodIssues,
setLogLevel: function (level) {
// Update the instance options object
const options = updateOptions(
validateOptions({
logging: {
level
}
})
);
const options = updateOptions({
logging: {
level
}
});

// Call the function
setLogLevel(options.logging.level);
},
enableConsoleLogging: function (toConsole) {
// Update the instance options object
const options = updateOptions(
validateOptions({
logging: {
toConsole
}
})
);
const options = updateOptions({
logging: {
toConsole
}
});

// Call the function
enableConsoleLogging(options.logging.toConsole);
},
enableFileLogging: function (dest, file, toFile) {
// Update the instance options object
const options = updateOptions(
validateOptions({
logging: {
dest,
file,
toFile
}
})
);
const options = updateOptions({
logging: {
dest,
file,
toFile
}
});

// Call the function
enableFileLogging(
Expand Down
8 changes: 4 additions & 4 deletions lib/server/middlewares/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ See LICENSE file in root for details.
import { v4 as uuid } from 'uuid';

import { getAllowCodeExecution } from '../../chart.js';
import { isAllowedConfig, validateOptions } from '../../config.js';
import { isAllowedConfig } from '../../config.js';
import { log } from '../../logger.js';
import { isObjectEmpty, isPrivateRangeUrlFound } from '../../utils.js';

Expand Down Expand Up @@ -146,8 +146,8 @@ function requestBodyMiddleware(request, response, next) {
);
}

// Validate the request options and store parsed structure in the request
request.validatedOptions = validateOptions({
// Get and pre-validate the options and store them in the request
request.validatedOptions = {
// Set the created ID as a `requestId` property in the options
requestId,
export: {
Expand Down Expand Up @@ -181,7 +181,7 @@ function requestBodyMiddleware(request, response, next) {
callback: body.callback,
resources: isAllowedConfig(body.resources, true, allowCodeExecution)
}
});
};

// Call the next middleware
return next();
Expand Down
13 changes: 1 addition & 12 deletions lib/server/routes/versionChange.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ See LICENSE file in root for details.
*/

import { getHighchartsVersion, updateHighchartsVersion } from '../../cache.js';
import { validateOption } from '../../config.js';
import { log } from '../../logger.js';
import { envs } from '../../validation.js';

Expand Down Expand Up @@ -63,17 +62,7 @@ export default function versionChangeRoutes(app) {
}

// Compare versions
let newVersion = request.params.newVersion;

// Validate the version
try {
newVersion = validateOption('version', request.params.newVersion);
} catch (error) {
throw new ExportError(
`[version] Version is incorrect: ${error.message}`,
400
).setError(error);
}
const newVersion = request.params.newVersion;

// When a correct value found
if (newVersion) {
Expand Down
22 changes: 9 additions & 13 deletions lib/server/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import http from 'http';
import https from 'https';
import multer from 'multer';

import { updateOptions, validateOptions } from '../config.js';
import { updateOptions } from '../config.js';
import { log, logWithStack } from '../logger.js';
import { __dirname, getAbsolutePath } from '../utils.js';

Expand Down Expand Up @@ -74,11 +74,9 @@ const app = express();
export async function startServer(serverOptions) {
try {
// Update the instance options object
const options = updateOptions(
validateOptions({
server: serverOptions
})
);
const options = updateOptions({
server: serverOptions
});

// Use validated options
serverOptions = options.server;
Expand Down Expand Up @@ -295,13 +293,11 @@ export function getApp() {
*/
export function enableRateLimiting(rateLimitingOptions) {
// Update the instance options object
const options = updateOptions(
validateOptions({
server: {
rateLimiting: rateLimitingOptions
}
})
);
const options = updateOptions({
server: {
rateLimiting: rateLimitingOptions
}
});

// Set the rate limiting options
rateLimitingMiddleware(app, options.server.rateLimitingOptions);
Expand Down

0 comments on commit ee8a7ea

Please sign in to comment.