From 825c8dec73aab1fbea3961b5181e84fcf9b07735 Mon Sep 17 00:00:00 2001
From: Bosco Domingo <boscodomingob@gmail.com>
Date: Sun, 19 May 2024 20:22:24 +0200
Subject: [PATCH] src: add `--env-file-if-exists` flag
Fixes: https://github.com/nodejs/node/issues/50993
Refs: https://github.com/nodejs/node/issues/51451
test: remove unnecessary comment
src: conform to style guidelines
src: change flag to `--env-file-optional`
test: revert automatic linter changes
doc: fix typos
src: change flag to `--env-file-if-exists`
src: refactor `env_file_data` and `GetEnvFileDataFromArgs`
test: clean up tests
src: print error when file not found
test: remove unnecessary extras
---
doc/api/cli.md | 16 ++++++++
src/node.cc | 18 ++++++---
src/node_dotenv.cc | 48 ++++++++++++++--------
src/node_dotenv.h | 6 ++-
src/node_options.cc | 4 ++
src/node_options.h | 1 +
test/parallel/test-dotenv-edge-cases.js | 54 ++++++++++++++++++++-----
7 files changed, 115 insertions(+), 32 deletions(-)
diff --git a/doc/api/cli.md b/doc/api/cli.md
index 01efb97e31be23..8de2f464c666ce 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -828,6 +828,8 @@ in the file, the value from the environment takes precedence.
You can pass multiple `--env-file` arguments. Subsequent files override
pre-existing variables defined in previous files.
+An error is thrown if the file does not exist.
+
```bash
node --env-file=.env --env-file=.development.env index.js
```
@@ -867,6 +869,9 @@ Export keyword before a key is ignored:
export USERNAME="nodejs" # will result in `nodejs` as the value.
```
+If you want to load environment variables from a file that may not exist, you
+can use the [`--env-file-if-exists`][] flag instead.
+
### `-e`, `--eval "script"`
<!-- YAML
@@ -1761,6 +1766,15 @@ is being linked to Node.js. Sharing the OpenSSL configuration may have unwanted
implications and it is recommended to use a configuration section specific to
Node.js which is `nodejs_conf` and is default when this option is not used.
+### `--env-file-if-exists=config`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+Behavior is the same as [`--env-file`][], but an error is not thrown if the file
+does not exist.
+
### `--pending-deprecation`
<!-- YAML
@@ -3548,6 +3562,8 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12
[`--build-snapshot`]: #--build-snapshot
[`--cpu-prof-dir`]: #--cpu-prof-dir
[`--diagnostic-dir`]: #--diagnostic-dirdirectory
+[`--env-file-if-exists`]: #--env-file-if-existsconfig
+[`--env-file`]: #--env-fileconfig
[`--experimental-default-type=module`]: #--experimental-default-typetype
[`--experimental-sea-config`]: single-executable-applications.md#generating-single-executable-preparation-blobs
[`--experimental-strip-types`]: #--experimental-strip-types
diff --git a/src/node.cc b/src/node.cc
index 50d1d17942194a..b9a92ade0b3ca5 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -854,20 +854,26 @@ static ExitCode InitializeNodeWithArgsInternal(
HandleEnvOptions(per_process::cli_options->per_isolate->per_env);
std::string node_options;
- auto file_paths = node::Dotenv::GetPathFromArgs(*argv);
+ auto env_files = node::Dotenv::GetDataFromArgs(*argv);
- if (!file_paths.empty()) {
+ if (!env_files.empty()) {
CHECK(!per_process::v8_initialized);
- for (const auto& file_path : file_paths) {
- switch (per_process::dotenv_file.ParsePath(file_path)) {
+ for (const auto& file_data : env_files) {
+ switch (per_process::dotenv_file.ParsePath(file_data.path)) {
case Dotenv::ParseResult::Valid:
break;
case Dotenv::ParseResult::InvalidContent:
- errors->push_back(file_path + ": invalid format");
+ errors->push_back(file_data.path + ": invalid format");
break;
case Dotenv::ParseResult::FileError:
- errors->push_back(file_path + ": not found");
+ if (file_data.is_optional) {
+ fprintf(stderr,
+ "%s not found. Continuing without it.\n",
+ file_data.path.c_str());
+ continue;
+ }
+ errors->push_back(file_data.path + ": not found");
break;
default:
UNREACHABLE();
diff --git a/src/node_dotenv.cc b/src/node_dotenv.cc
index 1cb57fcaea2628..f594df875d7a0c 100644
--- a/src/node_dotenv.cc
+++ b/src/node_dotenv.cc
@@ -11,36 +11,52 @@ using v8::NewStringType;
using v8::Object;
using v8::String;
-std::vector<std::string> Dotenv::GetPathFromArgs(
+std::vector<Dotenv::env_file_data> Dotenv::GetDataFromArgs(
const std::vector<std::string>& args) {
+ const std::string_view optional_env_file_flag = "--env-file-if-exists";
+
const auto find_match = [](const std::string& arg) {
- return arg == "--" || arg == "--env-file" || arg.starts_with("--env-file=");
+ return arg == "--" || arg == "--env-file" ||
+ arg.starts_with("--env-file=") || arg == "--env-file-if-exists" ||
+ arg.starts_with("--env-file-if-exists=");
};
- std::vector<std::string> paths;
- auto path = std::find_if(args.begin(), args.end(), find_match);
- while (path != args.end()) {
- if (*path == "--") {
- return paths;
+ std::vector<Dotenv::env_file_data> env_files;
+ // This will be an iterator, pointing to args.end() if no matches are found
+ auto matched_arg = std::find_if(args.begin(), args.end(), find_match);
+
+ while (matched_arg != args.end()) {
+ if (*matched_arg == "--") {
+ return env_files;
}
- auto equal_char = path->find('=');
- if (equal_char != std::string::npos) {
- paths.push_back(path->substr(equal_char + 1));
+ auto equal_char_index = matched_arg->find('=');
+
+ if (equal_char_index != std::string::npos) {
+ // `--env-file=path`
+ auto flag = matched_arg->substr(0, equal_char_index);
+ auto file_path = matched_arg->substr(equal_char_index + 1);
+
+ struct env_file_data env_file_data = {
+ file_path, flag.starts_with(optional_env_file_flag)};
+ env_files.push_back(env_file_data);
} else {
- auto next_path = std::next(path);
+ // `--env-file path`
+ auto file_path = std::next(matched_arg);
- if (next_path == args.end()) {
- return paths;
+ if (file_path == args.end()) {
+ return env_files;
}
- paths.push_back(*next_path);
+ struct env_file_data env_file_data = {
+ *file_path, matched_arg->starts_with(optional_env_file_flag)};
+ env_files.push_back(env_file_data);
}
- path = std::find_if(++path, args.end(), find_match);
+ matched_arg = std::find_if(++matched_arg, args.end(), find_match);
}
- return paths;
+ return env_files;
}
void Dotenv::SetEnvironment(node::Environment* env) {
diff --git a/src/node_dotenv.h b/src/node_dotenv.h
index ef9ee54a9e75ce..d508b13fc5db74 100644
--- a/src/node_dotenv.h
+++ b/src/node_dotenv.h
@@ -13,6 +13,10 @@ namespace node {
class Dotenv {
public:
enum ParseResult { Valid, FileError, InvalidContent };
+ struct env_file_data {
+ std::string path;
+ bool is_optional;
+ };
Dotenv() = default;
Dotenv(const Dotenv& d) = delete;
@@ -27,7 +31,7 @@ class Dotenv {
void SetEnvironment(Environment* env);
v8::Local<v8::Object> ToObject(Environment* env) const;
- static std::vector<std::string> GetPathFromArgs(
+ static std::vector<env_file_data> GetDataFromArgs(
const std::vector<std::string>& args);
private:
diff --git a/src/node_options.cc b/src/node_options.cc
index cc290661ec3b76..2ca6652538b30b 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -640,6 +640,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"set environment variables from supplied file",
&EnvironmentOptions::env_file);
Implies("--env-file", "[has_env_file_string]");
+ AddOption("--env-file-if-exists",
+ "set environment variables from supplied file",
+ &EnvironmentOptions::optional_env_file);
+ Implies("--env-file-if-exists", "[has_env_file_string]");
AddOption("--test",
"launch test runner on startup",
&EnvironmentOptions::test_runner);
diff --git a/src/node_options.h b/src/node_options.h
index 5f186ae86f0bcc..b521ca76185512 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -177,6 +177,7 @@ class EnvironmentOptions : public Options {
std::string redirect_warnings;
std::string diagnostic_dir;
std::string env_file;
+ std::string optional_env_file;
bool has_env_file_string = false;
bool test_runner = false;
uint64_t test_runner_concurrency = 0;
diff --git a/test/parallel/test-dotenv-edge-cases.js b/test/parallel/test-dotenv-edge-cases.js
index 5528d0b47fecb1..30fd9d20618ba0 100644
--- a/test/parallel/test-dotenv-edge-cases.js
+++ b/test/parallel/test-dotenv-edge-cases.js
@@ -10,30 +10,53 @@ const validEnvFilePath = '../fixtures/dotenv/valid.env';
const nodeOptionsEnvFilePath = '../fixtures/dotenv/node-options.env';
describe('.env supports edge cases', () => {
-
- it('supports multiple declarations', async () => {
- // process.env.BASIC is equal to `basic` because the second .env file overrides it.
+ it('supports multiple declarations, including optional ones', async () => {
const code = `
const assert = require('assert');
assert.strictEqual(process.env.BASIC, 'basic');
assert.strictEqual(process.env.NODE_NO_WARNINGS, '1');
`.trim();
+ const children = await Promise.all(Array.from({ length: 4 }, (_, i) =>
+ common.spawnPromisified(
+ process.execPath,
+ [
+ // Bitwise AND to create all 4 possible combinations:
+ // i & 0b01 is truthy when i has value 0bx1 (i.e. 0b01 (1) and 0b11 (3)), falsy otherwise.
+ // i & 0b10 is truthy when i has value 0b1x (i.e. 0b10 (2) and 0b11 (3)), falsy otherwise.
+ `${i & 0b01 ? '--env-file' : '--env-file-if-exists'}=${nodeOptionsEnvFilePath}`,
+ `${i & 0b10 ? '--env-file' : '--env-file-if-exists'}=${validEnvFilePath}`,
+ '--eval', code,
+ ],
+ { cwd: __dirname },
+ )));
+ assert.deepStrictEqual(children, Array.from({ length: 4 }, () => ({
+ code: 0,
+ signal: null,
+ stdout: '',
+ stderr: '',
+ })));
+ });
+
+ it('supports absolute paths', async () => {
+ const code = `
+ require('assert').strictEqual(process.env.BASIC, 'basic');
+ `.trim();
const child = await common.spawnPromisified(
process.execPath,
- [ `--env-file=${nodeOptionsEnvFilePath}`, `--env-file=${validEnvFilePath}`, '--eval', code ],
- { cwd: __dirname },
+ [ `--env-file=${path.resolve(__dirname, validEnvFilePath)}`, '--eval', code ],
);
assert.strictEqual(child.stderr, '');
assert.strictEqual(child.code, 0);
});
- it('supports absolute paths', async () => {
+ it('supports a space instead of \'=\' for the flag ', async () => {
const code = `
require('assert').strictEqual(process.env.BASIC, 'basic');
`.trim();
const child = await common.spawnPromisified(
process.execPath,
- [ `--env-file=${path.resolve(__dirname, validEnvFilePath)}`, '--eval', code ],
+ [ '--env-file', validEnvFilePath, '--eval', code ],
+ { cwd: __dirname },
);
assert.strictEqual(child.stderr, '');
assert.strictEqual(child.code, 0);
@@ -48,10 +71,23 @@ describe('.env supports edge cases', () => {
[ '--env-file=.env', '--eval', code ],
{ cwd: __dirname },
);
- assert.notStrictEqual(child.stderr.toString(), '');
+ assert.notStrictEqual(child.stderr, '');
assert.strictEqual(child.code, 9);
});
+ it('should handle non-existent optional .env file', async () => {
+ const code = `
+ require('assert').strictEqual(1,1);
+ `.trim();
+ const child = await common.spawnPromisified(
+ process.execPath,
+ ['--env-file-if-exists=.env', '--eval', code],
+ { cwd: __dirname },
+ );
+ assert.notStrictEqual(child.stderr, '');
+ assert.strictEqual(child.code, 0);
+ });
+
it('should not override existing environment variables but introduce new vars', async () => {
const code = `
require('assert').strictEqual(process.env.BASIC, 'existing');
@@ -106,7 +142,7 @@ describe('.env supports edge cases', () => {
'--eval', `require('assert').strictEqual(process.env.BASIC, undefined);`,
'--', '--env-file', validEnvFilePath,
],
- { cwd: fixtures.path('dotenv') },
+ { cwd: __dirname },
);
assert.strictEqual(child.stdout, '');
assert.strictEqual(child.stderr, '');