From e6a0d77d57a1dade108f05f1fe8b617af38d636f Mon Sep 17 00:00:00 2001 From: HEESEUNG Date: Sun, 6 Apr 2025 16:47:28 +0900 Subject: [PATCH] =?UTF-8?q?util:=20fix=20parseEnv=20incorrectly=20splittin?= =?UTF-8?q?g=20multiple=20=E2=80=98=3D=E2=80=98=20in=20value?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, parseEnv would create multiple environment variables if a single line contained multiple ‘=‘ characters (e.g. A=B=C would become { A: ‘B=C’, B: ‘C’ }). This commit ensures that only the first ‘=‘ is used as the key-value delimiter, and the rest of the line is treated as the value. Fixes: https://github.com/nodejs/node/issues/57411 PR-URL: https://github.com/nodejs/node/pull/57421 Reviewed-By: Daniel Lemire Reviewed-By: Matteo Collina --- benchmark/fixtures/valid.env | 2 ++ src/node_dotenv.cc | 38 ++++++++++++++++++++++------ test/fixtures/dotenv/valid.env | 2 ++ test/parallel/test-dotenv.js | 2 ++ test/parallel/test-util-parse-env.js | 2 ++ 5 files changed, 38 insertions(+), 8 deletions(-) diff --git a/benchmark/fixtures/valid.env b/benchmark/fixtures/valid.env index 120488d579..6df454da65 100644 --- a/benchmark/fixtures/valid.env +++ b/benchmark/fixtures/valid.env @@ -6,6 +6,8 @@ BASIC=basic # previous line intentionally left blank AFTER_LINE=after_line +A="B=C" +B=C=D EMPTY= EMPTY_SINGLE_QUOTES='' EMPTY_DOUBLE_QUOTES="" diff --git a/src/node_dotenv.cc b/src/node_dotenv.cc index 09b921fc81..dd66005811 100644 --- a/src/node_dotenv.cc +++ b/src/node_dotenv.cc @@ -145,7 +145,16 @@ void Dotenv::ParseContent(const std::string_view input) { // If there is no equal character, then ignore everything auto equal = content.find('='); if (equal == std::string_view::npos) { - break; + auto newline = content.find('\n'); + if (newline != std::string_view::npos) { + // If we used `newline` only, + // the '\n' might remain and cause an empty-line parse + content.remove_prefix(newline + 1); + } else { + content = {}; + } + // No valid data here, skip to next line + continue; } key = content.substr(0, equal); @@ -195,7 +204,9 @@ void Dotenv::ParseContent(const std::string_view input) { store_.insert_or_assign(std::string(key), multi_line_value); auto newline = content.find('\n', closing_quote + 1); if (newline != std::string_view::npos) { - content.remove_prefix(newline); + content.remove_prefix(newline + 1); + } else { + content = {}; } continue; } @@ -216,7 +227,7 @@ void Dotenv::ParseContent(const std::string_view input) { if (newline != std::string_view::npos) { value = content.substr(0, newline); store_.insert_or_assign(std::string(key), value); - content.remove_prefix(newline); + content.remove_prefix(newline + 1); } } else { // Example: KEY="value" @@ -226,8 +237,13 @@ void Dotenv::ParseContent(const std::string_view input) { // since there could be newline characters inside the value. auto newline = content.find('\n', closing_quote + 1); if (newline != std::string_view::npos) { - content.remove_prefix(newline); + // Use +1 to discard the '\n' itself => next line + content.remove_prefix(newline + 1); + } else { + content = {}; } + // No valid data here, skip to next line + continue; } } else { // Regular key value pair. @@ -243,15 +259,21 @@ void Dotenv::ParseContent(const std::string_view input) { if (hash_character != std::string_view::npos) { value = content.substr(0, hash_character); } - content.remove_prefix(newline); + store_.insert_or_assign(std::string(key), trim_spaces(value)); + content.remove_prefix(newline + 1); } else { // In case the last line is a single key/value pair // Example: KEY=VALUE (without a newline at the EOF) - value = content.substr(0); + value = content; + auto hash_char = value.find('#'); + if (hash_char != std::string_view::npos) { + value = content.substr(0, hash_char); + } + store_.insert_or_assign(std::string(key), trim_spaces(value)); + content = {}; } - value = trim_spaces(value); - store_.insert_or_assign(std::string(key), value); + store_.insert_or_assign(std::string(key), trim_spaces(value)); } } } diff --git a/test/fixtures/dotenv/valid.env b/test/fixtures/dotenv/valid.env index 120488d579..6df454da65 100644 --- a/test/fixtures/dotenv/valid.env +++ b/test/fixtures/dotenv/valid.env @@ -6,6 +6,8 @@ BASIC=basic # previous line intentionally left blank AFTER_LINE=after_line +A="B=C" +B=C=D EMPTY= EMPTY_SINGLE_QUOTES='' EMPTY_DOUBLE_QUOTES="" diff --git a/test/parallel/test-dotenv.js b/test/parallel/test-dotenv.js index 3c81bf9878..0115496875 100644 --- a/test/parallel/test-dotenv.js +++ b/test/parallel/test-dotenv.js @@ -82,3 +82,5 @@ assert.strictEqual(process.env.DONT_EXPAND_SQUOTED, 'dontexpand\\nnewlines'); assert.strictEqual(process.env.EXPORT_EXAMPLE, 'ignore export'); // Ignore spaces before double quotes to avoid quoted strings as value assert.strictEqual(process.env.SPACE_BEFORE_DOUBLE_QUOTES, 'space before double quotes'); +assert.strictEqual(process.env.A, 'B=C'); +assert.strictEqual(process.env.B, 'C=D'); diff --git a/test/parallel/test-util-parse-env.js b/test/parallel/test-util-parse-env.js index 13d2fda37a..80ab736dd3 100644 --- a/test/parallel/test-util-parse-env.js +++ b/test/parallel/test-util-parse-env.js @@ -11,6 +11,8 @@ const fs = require('node:fs'); const validContent = fs.readFileSync(validEnvFilePath, 'utf8'); assert.deepStrictEqual(util.parseEnv(validContent), { + A: 'B=C', + B: 'C=D', AFTER_LINE: 'after_line', BACKTICKS: 'backticks', BACKTICKS_INSIDE_DOUBLE: '`backticks` work inside double quotes',