Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 36.4k
util: fix parseEnv handling of invalid lines#57798
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -102,8 +102,11 @@ MaybeLocal<Object> Dotenv::ToObject(Environment* env) const { | ||
| return scope.Escape(result); | ||
| } | ||
| // Removes space characters (spaces, tabs and newlines) from | ||
| // the start and end of a given input string | ||
| // Removes leading and trailing spaces from a string_view. | ||
| // Returns an empty string_view if the input is empty. | ||
| // Example: | ||
| // trim_spaces(" hello ") -> "hello" | ||
| // trim_spaces("") -> "" | ||
| std::string_view trim_spaces(std::string_view input) { | ||
| if (input.empty()) return ""; | ||
| @@ -135,47 +138,62 @@ void Dotenv::ParseContent(const std::string_view input) { | ||
| while (!content.empty()) { | ||
| // Skip empty lines and comments | ||
| if (content.front() == '\n' || content.front() == '#') { | ||
| // Check if the first character of the content is a newline or a hash | ||
| auto newline = content.find('\n'); | ||
| if (newline != std::string_view::npos) { | ||
| content.remove_prefix(newline + 1); | ||
| continue; | ||
| } | ||
| } | ||
| // If there is no equal character, then ignore everything | ||
| auto equal = content.find('='); | ||
| if (equal == std::string_view::npos) { | ||
| 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 | ||
| // Remove everything up to and including the newline character | ||
| content.remove_prefix(newline + 1); | ||
| } else { | ||
| // If no newline is found, clear the content | ||
| content = {}; | ||
| } | ||
| // No valid data here, skip to next line | ||
| // Skip the remaining code in the loop and continue with the next | ||
| // iteration. | ||
| continue; | ||
| } | ||
| key = content.substr(0, equal); | ||
| content.remove_prefix(equal + 1); | ||
| // Find the next equals sign or newline in a single pass. | ||
| // This optimizes the search by avoiding multiple iterations. | ||
| auto equal_or_newline = content.find_first_of("=\n"); | ||
| // If we found nothing or found a newline before equals, the line is invalid | ||
| if (equal_or_newline == std::string_view::npos || | ||
| content.at(equal_or_newline) == '\n') { | ||
| if (equal_or_newline != std::string_view::npos) { | ||
| content.remove_prefix(equal_or_newline + 1); | ||
| content = trim_spaces(content); | ||
| continue; | ||
| } | ||
| break; | ||
| } | ||
| // We found an equals sign, extract the key | ||
| key = content.substr(0, equal_or_newline); | ||
| content.remove_prefix(equal_or_newline + 1); | ||
| key = trim_spaces(key); | ||
| // If the value is not present (e.g. KEY=) set is to an empty string | ||
| // If the value is not present (e.g. KEY=) set it to an empty string | ||
| if (content.empty() || content.front() == '\n') { | ||
| store_.insert_or_assign(std::string(key), ""); | ||
| continue; | ||
| } | ||
| content = trim_spaces(content); | ||
| if (key.empty()) { | ||
| break; | ||
| } | ||
| // Skip lines with empty keys after trimming spaces. | ||
| // Examples of invalid keys that would be skipped: | ||
| // =value | ||
| // " "=value | ||
| if (key.empty()) continue; | ||
AugustinMauroy marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| // Remove export prefix from key | ||
| // Remove export prefix from key and ensure proper spacing. | ||
| // Example: export FOO=bar -> FOO=bar | ||
| if (key.starts_with("export ")) { | ||
| key.remove_prefix(7); | ||
| // Trim spaces after removing export prefix to handle cases like: | ||
| // export FOO=bar | ||
| key = trim_spaces(key); | ||
AugustinMauroy marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| // SAFETY: Content is guaranteed to have at least one character | ||
| @@ -194,6 +212,7 @@ void Dotenv::ParseContent(const std::string_view input) { | ||
| value = content.substr(1, closing_quote - 1); | ||
| std::string multi_line_value = std::string(value); | ||
| // Replace \n with actual newlines in double-quoted strings | ||
| size_t pos = 0; | ||
| while ((pos = multi_line_value.find("\\n", pos)) != | ||
| std::string_view::npos) { | ||
| @@ -206,15 +225,17 @@ void Dotenv::ParseContent(const std::string_view input) { | ||
| if (newline != std::string_view::npos) { | ||
| 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 | ||
| content = {}; | ||
| } | ||
| continue; | ||
| } | ||
| } | ||
| // Check if the value is wrapped in quotes, single quotes or backticks | ||
| if ((content.front() == '\'' || content.front() == '"' || | ||
| content.front() == '`')) { | ||
| // Handle quoted values (single quotes, double quotes, backticks) | ||
| if (content.front() == '\'' || content.front() == '"' || | ||
| content.front() == '`') { | ||
| auto closing_quote = content.find(content.front(), 1); | ||
| // Check if the closing quote is not found | ||
| @@ -228,13 +249,16 @@ void Dotenv::ParseContent(const std::string_view input) { | ||
| value = content.substr(0, newline); | ||
| store_.insert_or_assign(std::string(key), value); | ||
| content.remove_prefix(newline + 1); | ||
| } else { | ||
| // No newline - take rest of content | ||
| value = content; | ||
| store_.insert_or_assign(std::string(key), value); | ||
| break; | ||
AugustinMauroy marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| } else { | ||
| // Example: KEY="value" | ||
| // Found closing quote - take content between quotes | ||
| value = content.substr(1, closing_quote - 1); | ||
| store_.insert_or_assign(std::string(key), value); | ||
| // Select the first newline after the closing quotation mark | ||
| // since there could be newline characters inside the value. | ||
| auto newline = content.find('\n', closing_quote + 1); | ||
| if (newline != std::string_view::npos) { | ||
| // Use +1 to discard the '\n' itself => next line | ||
| @@ -257,13 +281,13 @@ void Dotenv::ParseContent(const std::string_view input) { | ||
| // Example: KEY=value # comment | ||
| // The value pair should be `value` | ||
| if (hash_character != std::string_view::npos) { | ||
| value = content.substr(0, hash_character); | ||
| value = value.substr(0, hash_character); | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you make this change? Please add a test that covers this. | ||
| } | ||
| store_.insert_or_assign(std::string(key), trim_spaces(value)); | ||
| value = trim_spaces(value); | ||
AugustinMauroy marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| store_.insert_or_assign(std::string(key), std::string(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) | ||
AugustinMauroy marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| // Last line without newline | ||
| value = content; | ||
| auto hash_char = value.find('#'); | ||
| if (hash_char != std::string_view::npos) { | ||
| @@ -272,9 +296,9 @@ void Dotenv::ParseContent(const std::string_view input) { | ||
| store_.insert_or_assign(std::string(key), trim_spaces(value)); | ||
| content = {}; | ||
| } | ||
| store_.insert_or_assign(std::string(key), trim_spaces(value)); | ||
| } | ||
| content = trim_spaces(content); | ||
AugustinMauroy marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -4,6 +4,7 @@ const common = require('../common'); | ||
| const assert = require('node:assert'); | ||
| const path = require('node:path'); | ||
| const { describe, it } = require('node:test'); | ||
| const { parseEnv } = require('node:util'); | ||
| const fixtures = require('../common/fixtures'); | ||
| const validEnvFilePath = '../fixtures/dotenv/valid.env'; | ||
| @@ -200,4 +201,61 @@ describe('.env supports edge cases', () => { | ||
| assert.strictEqual(child.code, 9); | ||
| assert.match(child.stderr, /bad option: --env-file-ABCD/); | ||
| }); | ||
| it('should handle invalid multiline syntax', () => { | ||
AugustinMauroy marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| const result = parseEnv([ | ||
| 'foo', | ||
| '', | ||
| 'bar', | ||
| 'baz=whatever', | ||
| 'VALID_AFTER_INVALID=test', | ||
| 'multiple_invalid', | ||
| 'lines_without_equals', | ||
| 'ANOTHER_VALID=value', | ||
| ].join('\n')); | ||
| assert.deepStrictEqual(result, { | ||
| baz: 'whatever', | ||
| VALID_AFTER_INVALID: 'test', | ||
| ANOTHER_VALID: 'value', | ||
| }); | ||
| }); | ||
| it('should handle trimming of keys and values correctly', () => { | ||
| const result = parseEnv([ | ||
| ' KEY_WITH_SPACES_BEFORE= value_with_spaces_before_and_after ', | ||
| 'KEY_WITH_TABS_BEFORE\t=\tvalue_with_tabs_before_and_after\t', | ||
| 'KEY_WITH_SPACES_AND_TABS\t = \t value_with_spaces_and_tabs \t', | ||
| ' KEY_WITH_SPACES_ONLY =value', | ||
| 'KEY_WITH_NO_VALUE=', | ||
| 'KEY_WITH_SPACES_AFTER= value ', | ||
| 'KEY_WITH_SPACES_AND_COMMENT=value # this is a comment', | ||
| 'KEY_WITH_ONLY_COMMENT=# this is a comment', | ||
| 'KEY_WITH_EXPORT=export value', | ||
| ' export KEY_WITH_EXPORT_AND_SPACES = value ', | ||
| ].join('\n')); | ||
| assert.deepStrictEqual(result, { | ||
| KEY_WITH_SPACES_BEFORE: 'value_with_spaces_before_and_after', | ||
| KEY_WITH_TABS_BEFORE: 'value_with_tabs_before_and_after', | ||
| KEY_WITH_SPACES_AND_TABS: 'value_with_spaces_and_tabs', | ||
| KEY_WITH_SPACES_ONLY: 'value', | ||
| KEY_WITH_NO_VALUE: '', | ||
| KEY_WITH_ONLY_COMMENT: '', | ||
| KEY_WITH_SPACES_AFTER: 'value', | ||
| KEY_WITH_SPACES_AND_COMMENT: 'value', | ||
| KEY_WITH_EXPORT: 'export value', | ||
| KEY_WITH_EXPORT_AND_SPACES: 'value', | ||
| }); | ||
| }); | ||
| it('should handle a comment in a valid value', () => { | ||
| const result = parseEnv([ | ||
| 'KEY_WITH_COMMENT_IN_VALUE="value # this is a comment"', | ||
| ].join('\n')); | ||
| assert.deepStrictEqual(result, { | ||
| KEY_WITH_COMMENT_IN_VALUE: 'value # this is a comment', | ||
| }); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not new to this PR but it occurs to me that this function overlooks the often-overlooked vertical tab (0x0B).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I maybe can help on that, but I don't know what your are talking about