Skip to content

567/user-data-dir-config #630

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Feb 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .env.sample
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# PUPPETEER CONFIG
PUPPETEER_TEMP_DIR = ./tmp/

# HIGHCHARTS CONFIG
HIGHCHARTS_VERSION = latest
HIGHCHARTS_CDN_URL = https://code.highcharts.com/
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ _New Features:_

- Added proxy authentication [(#631)](https://github.com/highcharts/node-export-server/issues/631).

_New features:_

- Made the temporary Puppeteer directory (`PUPPETEER_TEMP_DIR`) (till now, `'./tmp'`) configurable by the user [(#567)](https://github.com/highcharts/node-export-server/issues/567).

# 4.0.2

_Hotfix_:
Expand Down
7 changes: 6 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ The format, along with its default values, is as follows (using the recommended
```
{
"puppeteer": {
"args": []
"args": [],
"tempDir": "./tmp/"
},
"highcharts": {
"version": "latest",
Expand Down Expand Up @@ -288,6 +289,10 @@ To load an additional JSON configuration file, use the `--loadConfig <filepath>`

These variables are set in your environment and take precedence over options from the `lib/schemas/config.js` file. They can be set in the `.env` file (refer to the `.env.sample` file). If you prefer setting these variables through the `package.json`, use `export` command on Linux/Mac OS X and `set` command on Windows.

### Puppeteer Config

- `PUPPETEER_TEMP_DIR`: The directory for Puppeteer to store temporary files (defaults to `./tmp/`).

### Highcharts Config

- `HIGHCHARTS_VERSION`: Highcharts version to use (defaults to `latest`).
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.

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

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions lib/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ export function get() {
*/
export async function create(puppeteerArgs) {
// Get debug and other options
const { debug, other } = getOptions();
const { puppeteer: puppeteerOptions, debug, other } = getOptions();

// Get the debug options
const { enable: enabledDebug, ...debugOptions } = debug;

const launchOptions = {
headless: other.browserShellMode ? 'shell' : true,
userDataDir: './tmp/',
userDataDir: puppeteerOptions.tempDir || './tmp/',
args: puppeteerArgs,
handleSIGINT: false,
handleSIGTERM: false,
Expand Down
21 changes: 21 additions & 0 deletions lib/envs.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,24 @@
)
.transform((value) => (value !== '' ? value : undefined)),

// Checks if the string is a valid path directory (path format)
path: () =>
z
.string()
.trim()
.refine(
(value) => {
// Simplified regex to match both absolute and relative paths
return /^(\.\/|\.\.\/|\/|[a-zA-Z]:\\|[a-zA-Z]:\/)?(([\w]+|-)[\\/]?)+$/.test(

Check failure

Code scanning / CodeQL

Inefficient regular expression High

This part of the regular expression may cause exponential backtracking on strings containing many repetitions of '0'.

Copilot Autofix

AI 3 months ago

To fix the problem, we need to modify the regular expression to remove the ambiguity that can lead to exponential backtracking. Specifically, we should replace the [\w]+ pattern with a more precise pattern that avoids ambiguity. One way to achieve this is to use a non-capturing group with a negated character class to ensure that each character is matched in a deterministic way.

  • Replace the [\w]+ pattern with (?:[\w-]+) to ensure that the pattern matches sequences of word characters and hyphens without causing exponential backtracking.
  • Update the regular expression in the path validator function in the lib/envs.js file.
Suggested changeset 1
lib/envs.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/lib/envs.js b/lib/envs.js
--- a/lib/envs.js
+++ b/lib/envs.js
@@ -74,3 +74,3 @@
           // Simplified regex to match both absolute and relative paths
-          return /^(\.\/|\.\.\/|\/|[a-zA-Z]:\\|[a-zA-Z]:\/)?(([\w]+|-)[\\/]?)+$/.test(
+          return /^(\.\/|\.\.\/|\/|[a-zA-Z]:\\|[a-zA-Z]:\/)?((?:[\w-]+)[\\/]?)+$/.test(
             value
EOF
@@ -74,3 +74,3 @@
// Simplified regex to match both absolute and relative paths
return /^(\.\/|\.\.\/|\/|[a-zA-Z]:\\|[a-zA-Z]:\/)?(([\w]+|-)[\\/]?)+$/.test(
return /^(\.\/|\.\.\/|\/|[a-zA-Z]:\\|[a-zA-Z]:\/)?((?:[\w-]+)[\\/]?)+$/.test(
value
Copilot is powered by AI and may make mistakes. Always verify output.
@PaulDalek PaulDalek committed this autofix suggestion 3 months ago.
value
);
},
{},
{
message: 'The string is an invalid path directory string.'
}
),

// Allows positive numbers or no value in which case the returned value will
// be undefined
positiveNum: () =>
Expand Down Expand Up @@ -96,6 +114,9 @@
};

export const Config = z.object({
// puppeteer
PUPPETEER_TEMP_DIR: v.string(),

// highcharts
HIGHCHARTS_VERSION: z
.string()
Expand Down
7 changes: 6 additions & 1 deletion lib/schemas/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,12 @@ export const defaultConfig = {
],
type: 'string[]',
description: 'Arguments array to send to Puppeteer.'
},
tempDir: {
value: './tmp/',
type: 'string',
envLink: 'PUPPETEER_TEMP_DIR',
description: 'The directory for Puppeteer to store temporary files.'
}
},
highcharts: {
Expand Down Expand Up @@ -355,7 +361,6 @@ export const defaultConfig = {
envLink: 'SERVER_MAX_UPLOAD_SIZE',
description:
'The maximum upload size, in megabytes, for the server'

},
enable: {
value: false,
Expand Down
25 changes: 25 additions & 0 deletions tests/unit/envs.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,31 @@
import { Config } from '../../lib/envs';

describe('Environment variables should be correctly parsed', () => {
test('PUPPETEER_TEMP_DIR should be a valid path', () => {
const env = { PUPPETEER_TEMP_DIR: '/path/to/dir' };
expect(Config.partial().parse(env).PUPPETEER_TEMP_DIR).toEqual(
'/path/to/dir'
);

env.PUPPETEER_TEMP_DIR = '/another/path/to/dir';
expect(Config.partial().parse(env).PUPPETEER_TEMP_DIR).toEqual(
'/another/path/to/dir'
);

env.PUPPETEER_TEMP_DIR = '';
expect(() => Config.partial().parse(env)).toThrow();
});

test('PUPPETEER_TEMP_DIR can be a relative path', () => {
const env = { PUPPETEER_TEMP_DIR: './tmp/' };
expect(Config.partial().parse(env).PUPPETEER_TEMP_DIR).toEqual('./tmp/');

env.PUPPETEER_TEMP_DIR = '../custom-tmp/';
expect(Config.partial().parse(env).PUPPETEER_TEMP_DIR).toEqual(
'../custom-tmp/'
);
});

test('HIGHCHARTS_VERSION accepts latests and not unrelated strings', () => {
const env = { HIGHCHARTS_VERSION: 'string-other-than-latest' };
expect(() => Config.partial().parse(env)).toThrow();
Expand Down
Loading