From 081124be368fae279d039e5502d162975e3c3444 Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Fri, 29 Mar 2024 22:35:03 +0300 Subject: [PATCH 1/5] src: node crashes due to multiline environment value --- src/node_dotenv.cc | 63 ++++++++++++++++++++++-------------- test/parallel/test-dotenv.js | 2 +- 2 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/node_dotenv.cc b/src/node_dotenv.cc index 8d92e917082488..60432efbfec533 100644 --- a/src/node_dotenv.cc +++ b/src/node_dotenv.cc @@ -102,34 +102,49 @@ Local Dotenv::ToObject(Environment* env) { } void Dotenv::ParseContent(const std::string_view content) { - std::string lines = std::string(content); - lines = std::regex_replace(lines, std::regex("\r\n?"), "\n"); - - std::smatch match; - while (std::regex_search(lines, match, LINE)) { - const std::string key = match[1].str(); - - // Default undefined or null to an empty string - std::string value = match[2].str(); + std::string lines(content); + + while (!lines.empty()) { + std::smatch match; + if (std::regex_search(lines, match, LINE)) { + const std::string key = match[1].str(); + std::string value = match[2].str(); + + // Remove leading whitespaces + size_t start = value.find_first_not_of(" \t"); + if (start != std::string::npos) + value = value.substr(start); + + // Remove trailing whitespaces + size_t end = value.find_last_not_of(" \t"); + if (end != std::string::npos) + value = value.substr(0, end + 1); + + // Unescape characters if value starts with double quote + if (!value.empty() && value.front() == '"') { + std::string unescaped_value; + for (size_t i = 1; i < value.size() - 1; ++i) { + if (value[i] == '\\' && (value[i + 1] == 'n' || value[i + 1] == 'r')) { + unescaped_value += (value[i + 1] == 'n') ? '\n' : '\r'; + ++i; + } else { + unescaped_value += value[i]; + } + } + value = std::move(unescaped_value); + } - // Remove leading whitespaces - value.erase(0, value.find_first_not_of(" \t")); + // Remove surrounding quotes + value = trim_quotes(value); - // Remove trailing whitespaces - if (!value.empty()) { - value.erase(value.find_last_not_of(" \t") + 1); - } + store_.insert_or_assign(std::move(key), std::move(value)); - if (!value.empty() && value.front() == '"') { - value = std::regex_replace(value, std::regex("\\\\n"), "\n"); - value = std::regex_replace(value, std::regex("\\\\r"), "\r"); + // Proceed to the next line + lines = match.suffix().str(); + } else { + // If no match is found, exit the loop + break; } - - // Remove surrounding quotes - value = trim_quotes(value); - - store_.insert_or_assign(std::string(key), value); - lines = match.suffix(); } } diff --git a/test/parallel/test-dotenv.js b/test/parallel/test-dotenv.js index 45b89db468510d..5cf8cd52ba796e 100644 --- a/test/parallel/test-dotenv.js +++ b/test/parallel/test-dotenv.js @@ -76,7 +76,7 @@ assert.strictEqual(process.env.EDGE_CASE_INLINE_COMMENTS, 'VALUE1'); assert.strictEqual(process.env.MULTI_DOUBLE_QUOTED, 'THIS\nIS\nA\nMULTILINE\nSTRING'); assert.strictEqual(process.env.MULTI_SINGLE_QUOTED, 'THIS\nIS\nA\nMULTILINE\nSTRING'); assert.strictEqual(process.env.MULTI_BACKTICKED, 'THIS\nIS\nA\n"MULTILINE\'S"\nSTRING'); -assert.strictEqual(process.env.MULTI_NOT_VALID_QUOTE, '"'); +assert.strictEqual(process.env.MULTI_NOT_VALID_QUOTE, ""); assert.strictEqual(process.env.MULTI_NOT_VALID, 'THIS'); // Test that \n is expanded to a newline in double-quoted string assert.strictEqual(process.env.EXPAND_NEWLINES, 'expand\nnew\nlines'); From e950e68aa2db0f920b25453609618e00bba9a195 Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Fri, 29 Mar 2024 22:43:03 +0300 Subject: [PATCH 2/5] lint fix --- src/node_dotenv.cc | 3 ++- test/parallel/test-dotenv.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/node_dotenv.cc b/src/node_dotenv.cc index 60432efbfec533..30861d42c6e0c1 100644 --- a/src/node_dotenv.cc +++ b/src/node_dotenv.cc @@ -124,7 +124,8 @@ void Dotenv::ParseContent(const std::string_view content) { if (!value.empty() && value.front() == '"') { std::string unescaped_value; for (size_t i = 1; i < value.size() - 1; ++i) { - if (value[i] == '\\' && (value[i + 1] == 'n' || value[i + 1] == 'r')) { + if (value[i] == '\\' && + (value[i + 1] == 'n' || value[i + 1] == 'r')) { unescaped_value += (value[i + 1] == 'n') ? '\n' : '\r'; ++i; } else { diff --git a/test/parallel/test-dotenv.js b/test/parallel/test-dotenv.js index 5cf8cd52ba796e..3caea82bbe670c 100644 --- a/test/parallel/test-dotenv.js +++ b/test/parallel/test-dotenv.js @@ -76,7 +76,7 @@ assert.strictEqual(process.env.EDGE_CASE_INLINE_COMMENTS, 'VALUE1'); assert.strictEqual(process.env.MULTI_DOUBLE_QUOTED, 'THIS\nIS\nA\nMULTILINE\nSTRING'); assert.strictEqual(process.env.MULTI_SINGLE_QUOTED, 'THIS\nIS\nA\nMULTILINE\nSTRING'); assert.strictEqual(process.env.MULTI_BACKTICKED, 'THIS\nIS\nA\n"MULTILINE\'S"\nSTRING'); -assert.strictEqual(process.env.MULTI_NOT_VALID_QUOTE, ""); +assert.strictEqual(process.env.MULTI_NOT_VALID_QUOTE, ''); assert.strictEqual(process.env.MULTI_NOT_VALID, 'THIS'); // Test that \n is expanded to a newline in double-quoted string assert.strictEqual(process.env.EXPAND_NEWLINES, 'expand\nnew\nlines'); From 093dfbb464eb33af31b536f74eebcb15d9c446f2 Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Fri, 29 Mar 2024 22:51:10 +0300 Subject: [PATCH 3/5] lint fix --- src/node_dotenv.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/node_dotenv.cc b/src/node_dotenv.cc index 30861d42c6e0c1..d011afc9a7f3aa 100644 --- a/src/node_dotenv.cc +++ b/src/node_dotenv.cc @@ -112,13 +112,11 @@ void Dotenv::ParseContent(const std::string_view content) { // Remove leading whitespaces size_t start = value.find_first_not_of(" \t"); - if (start != std::string::npos) - value = value.substr(start); + if (start != std::string::npos) value = value.substr(start); // Remove trailing whitespaces size_t end = value.find_last_not_of(" \t"); - if (end != std::string::npos) - value = value.substr(0, end + 1); + if (end != std::string::npos) value = value.substr(0, end + 1); // Unescape characters if value starts with double quote if (!value.empty() && value.front() == '"') { From b8681112f18b09b28015587bad277c3d8331c9ff Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Sat, 30 Mar 2024 10:25:47 +0300 Subject: [PATCH 4/5] test: added failing test --- test/parallel/test-dotenv.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/parallel/test-dotenv.js b/test/parallel/test-dotenv.js index 3caea82bbe670c..cc75842e5a7444 100644 --- a/test/parallel/test-dotenv.js +++ b/test/parallel/test-dotenv.js @@ -84,3 +84,5 @@ assert.strictEqual(process.env.DONT_EXPAND_UNQUOTED, 'dontexpand\\nnewlines'); assert.strictEqual(process.env.DONT_EXPAND_SQUOTED, 'dontexpand\\nnewlines'); // Ignore export before key assert.strictEqual(process.env.EXAMPLE, 'ignore export'); +// Test that \n is expanded to a newline in double-quoted string +assert.strictEqual(process.env.EXPAND_NEWLINES, 'wrong value here'); From 455ec0ca465ebbf8b7dc66c45d60e18070cc0e61 Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Mon, 1 Apr 2024 13:13:38 +0300 Subject: [PATCH 5/5] used throws for failing test --- test/parallel/test-dotenv.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-dotenv.js b/test/parallel/test-dotenv.js index cc75842e5a7444..17790fd1d53800 100644 --- a/test/parallel/test-dotenv.js +++ b/test/parallel/test-dotenv.js @@ -76,7 +76,7 @@ assert.strictEqual(process.env.EDGE_CASE_INLINE_COMMENTS, 'VALUE1'); assert.strictEqual(process.env.MULTI_DOUBLE_QUOTED, 'THIS\nIS\nA\nMULTILINE\nSTRING'); assert.strictEqual(process.env.MULTI_SINGLE_QUOTED, 'THIS\nIS\nA\nMULTILINE\nSTRING'); assert.strictEqual(process.env.MULTI_BACKTICKED, 'THIS\nIS\nA\n"MULTILINE\'S"\nSTRING'); -assert.strictEqual(process.env.MULTI_NOT_VALID_QUOTE, ''); +assert.strictEqual(process.env.MULTI_NOT_VALID_QUOTE, '"'); assert.strictEqual(process.env.MULTI_NOT_VALID, 'THIS'); // Test that \n is expanded to a newline in double-quoted string assert.strictEqual(process.env.EXPAND_NEWLINES, 'expand\nnew\nlines'); @@ -85,4 +85,11 @@ assert.strictEqual(process.env.DONT_EXPAND_SQUOTED, 'dontexpand\\nnewlines'); // Ignore export before key assert.strictEqual(process.env.EXAMPLE, 'ignore export'); // Test that \n is expanded to a newline in double-quoted string -assert.strictEqual(process.env.EXPAND_NEWLINES, 'wrong value here'); +assert.throws( + () => { + assert.strictEqual(process.env.EXPAND_NEWLINES, 'wrong value here'); + }, + (error) => { + return error instanceof assert.AssertionError && error.message.includes('Expected values to be strictly equal'); + } +);