FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

util: fix parseEnv handling of invalid lines · nodejs/node@2df7ce9 · GitHub

/ node Public

Commit 2df7ce9

Browse files
AugustinMauroy
authored and
aduh95
committed
util: fix parseEnv handling of invalid lines
PR-URL: #57798 Fixes: #56775 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
1 parent f210a15 commit 2df7ce9

2 files changed

Lines changed: 116 additions & 34 deletions

File tree

‎src/node_dotenv.cc‎

Lines changed: 58 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,11 @@ MaybeLocal<Object> Dotenv::ToObject(Environment* env) const {
102102
return scope.Escape(result);
103103
}
104104

105-
// Removes space characters (spaces, tabs and newlines) from
106-
// the start and end of a given input string
105+
// Removes leading and trailing spaces from a string_view.
106+
// Returns an empty string_view if the input is empty.
107+
// Example:
108+
// trim_spaces(" hello ") -> "hello"
109+
// trim_spaces("") -> ""
107110
std::string_view trim_spaces(std::string_view input) {
108111
if (input.empty()) return "";
109112

@@ -135,47 +138,62 @@ void Dotenv::ParseContent(const std::string_view input) {
135138
while (!content.empty()) {
136139
// Skip empty lines and comments
137140
if (content.front() == '\n' || content.front() == '#') {
141+
// Check if the first character of the content is a newline or a hash
138142
auto newline = content.find('\n');
139143
if (newline != std::string_view::npos) {
140-
content.remove_prefix(newline + 1);
141-
continue;
142-
}
143-
}
144-
145-
// If there is no equal character, then ignore everything
146-
auto equal = content.find('=');
147-
if (equal == std::string_view::npos) {
148-
auto newline = content.find('\n');
149-
if (newline != std::string_view::npos) {
150-
// If we used `newline` only,
151-
// the '\n' might remain and cause an empty-line parse
144+
// Remove everything up to and including the newline character
152145
content.remove_prefix(newline + 1);
153146
} else {
147+
// If no newline is found, clear the content
154148
content = {};
155149
}
156-
// No valid data here, skip to next line
150+
151+
// Skip the remaining code in the loop and continue with the next
152+
// iteration.
157153
continue;
158154
}
159155

160-
key = content.substr(0, equal);
161-
content.remove_prefix(equal + 1);
156+
// Find the next equals sign or newline in a single pass.
157+
// This optimizes the search by avoiding multiple iterations.
158+
auto equal_or_newline = content.find_first_of("=\n");
159+
160+
// If we found nothing or found a newline before equals, the line is invalid
161+
if (equal_or_newline == std::string_view::npos ||
162+
content.at(equal_or_newline) == '\n') {
163+
if (equal_or_newline != std::string_view::npos) {
164+
content.remove_prefix(equal_or_newline + 1);
165+
content = trim_spaces(content);
166+
continue;
167+
}
168+
break;
169+
}
170+
171+
// We found an equals sign, extract the key
172+
key = content.substr(0, equal_or_newline);
173+
content.remove_prefix(equal_or_newline + 1);
162174
key = trim_spaces(key);
163175

164-
// If the value is not present (e.g. KEY=) set is to an empty string
176+
// If the value is not present (e.g. KEY=) set it to an empty string
165177
if (content.empty() || content.front() == '\n') {
166178
store_.insert_or_assign(std::string(key), "");
167179
continue;
168180
}
169181

170182
content = trim_spaces(content);
171183

172-
if (key.empty()) {
173-
break;
174-
}
184+
// Skip lines with empty keys after trimming spaces.
185+
// Examples of invalid keys that would be skipped:
186+
// =value
187+
// " "=value
188+
if (key.empty()) continue;
175189

176-
// Remove export prefix from key
190+
// Remove export prefix from key and ensure proper spacing.
191+
// Example: export FOO=bar -> FOO=bar
177192
if (key.starts_with("export ")) {
178193
key.remove_prefix(7);
194+
// Trim spaces after removing export prefix to handle cases like:
195+
// export FOO=bar
196+
key = trim_spaces(key);
179197
}
180198

181199
// SAFETY: Content is guaranteed to have at least one character
@@ -194,6 +212,7 @@ void Dotenv::ParseContent(const std::string_view input) {
194212
value = content.substr(1, closing_quote - 1);
195213
std::string multi_line_value = std::string(value);
196214

215+
// Replace \n with actual newlines in double-quoted strings
197216
size_t pos = 0;
198217
while ((pos = multi_line_value.find("\\n", pos)) !=
199218
std::string_view::npos) {
@@ -206,15 +225,17 @@ void Dotenv::ParseContent(const std::string_view input) {
206225
if (newline != std::string_view::npos) {
207226
content.remove_prefix(newline + 1);
208227
} else {
228+
// In case the last line is a single key/value pair
229+
// Example: KEY=VALUE (without a newline at the EOF
209230
content = {};
210231
}
211232
continue;
212233
}
213234
}
214235

215-
// Check if the value is wrapped in quotes, single quotes or backticks
216-
if ((content.front() == '\'' || content.front() == '"' ||
217-
content.front() == '`')) {
236+
// Handle quoted values (single quotes, double quotes, backticks)
237+
if (content.front() == '\'' || content.front() == '"' ||
238+
content.front() == '`') {
218239
auto closing_quote = content.find(content.front(), 1);
219240

220241
// Check if the closing quote is not found
@@ -228,13 +249,16 @@ void Dotenv::ParseContent(const std::string_view input) {
228249
value = content.substr(0, newline);
229250
store_.insert_or_assign(std::string(key), value);
230251
content.remove_prefix(newline + 1);
252+
} else {
253+
// No newline - take rest of content
254+
value = content;
255+
store_.insert_or_assign(std::string(key), value);
256+
break;
231257
}
232258
} else {
233-
// Example: KEY="value"
259+
// Found closing quote - take content between quotes
234260
value = content.substr(1, closing_quote - 1);
235261
store_.insert_or_assign(std::string(key), value);
236-
// Select the first newline after the closing quotation mark
237-
// since there could be newline characters inside the value.
238262
auto newline = content.find('\n', closing_quote + 1);
239263
if (newline != std::string_view::npos) {
240264
// Use +1 to discard the '\n' itself => next line
@@ -257,13 +281,13 @@ void Dotenv::ParseContent(const std::string_view input) {
257281
// Example: KEY=value # comment
258282
// The value pair should be `value`
259283
if (hash_character != std::string_view::npos) {
260-
value = content.substr(0, hash_character);
284+
value = value.substr(0, hash_character);
261285
}
262-
store_.insert_or_assign(std::string(key), trim_spaces(value));
286+
value = trim_spaces(value);
287+
store_.insert_or_assign(std::string(key), std::string(value));
263288
content.remove_prefix(newline + 1);
264289
} else {
265-
// In case the last line is a single key/value pair
266-
// Example: KEY=VALUE (without a newline at the EOF)
290+
// Last line without newline
267291
value = content;
268292
auto hash_char = value.find('#');
269293
if (hash_char != std::string_view::npos) {
@@ -272,9 +296,9 @@ void Dotenv::ParseContent(const std::string_view input) {
272296
store_.insert_or_assign(std::string(key), trim_spaces(value));
273297
content = {};
274298
}
275-
276-
store_.insert_or_assign(std::string(key), trim_spaces(value));
277299
}
300+
301+
content = trim_spaces(content);
278302
}
279303
}
280304

‎test/parallel/test-dotenv-edge-cases.js‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const common = require('../common');
44
const assert = require('node:assert');
55
const path = require('node:path');
66
const { describe, it } = require('node:test');
7+
const { parseEnv } = require('node:util');
78
const fixtures = require('../common/fixtures');
89

910
const validEnvFilePath = '../fixtures/dotenv/valid.env';
@@ -200,4 +201,61 @@ describe('.env supports edge cases', () => {
200201
assert.strictEqual(child.code, 9);
201202
assert.match(child.stderr, /bad option: --env-file-ABCD/);
202203
});
204+
205+
it('should handle invalid multiline syntax', () => {
206+
const result = parseEnv([
207+
'foo',
208+
'',
209+
'bar',
210+
'baz=whatever',
211+
'VALID_AFTER_INVALID=test',
212+
'multiple_invalid',
213+
'lines_without_equals',
214+
'ANOTHER_VALID=value',
215+
].join('\n'));
216+
217+
assert.deepStrictEqual(result, {
218+
baz: 'whatever',
219+
VALID_AFTER_INVALID: 'test',
220+
ANOTHER_VALID: 'value',
221+
});
222+
});
223+
224+
it('should handle trimming of keys and values correctly', () => {
225+
const result = parseEnv([
226+
' KEY_WITH_SPACES_BEFORE= value_with_spaces_before_and_after ',
227+
'KEY_WITH_TABS_BEFORE\t=\tvalue_with_tabs_before_and_after\t',
228+
'KEY_WITH_SPACES_AND_TABS\t = \t value_with_spaces_and_tabs \t',
229+
' KEY_WITH_SPACES_ONLY =value',
230+
'KEY_WITH_NO_VALUE=',
231+
'KEY_WITH_SPACES_AFTER= value ',
232+
'KEY_WITH_SPACES_AND_COMMENT=value # this is a comment',
233+
'KEY_WITH_ONLY_COMMENT=# this is a comment',
234+
'KEY_WITH_EXPORT=export value',
235+
' export KEY_WITH_EXPORT_AND_SPACES = value ',
236+
].join('\n'));
237+
238+
assert.deepStrictEqual(result, {
239+
KEY_WITH_SPACES_BEFORE: 'value_with_spaces_before_and_after',
240+
KEY_WITH_TABS_BEFORE: 'value_with_tabs_before_and_after',
241+
KEY_WITH_SPACES_AND_TABS: 'value_with_spaces_and_tabs',
242+
KEY_WITH_SPACES_ONLY: 'value',
243+
KEY_WITH_NO_VALUE: '',
244+
KEY_WITH_ONLY_COMMENT: '',
245+
KEY_WITH_SPACES_AFTER: 'value',
246+
KEY_WITH_SPACES_AND_COMMENT: 'value',
247+
KEY_WITH_EXPORT: 'export value',
248+
KEY_WITH_EXPORT_AND_SPACES: 'value',
249+
});
250+
});
251+
252+
it('should handle a comment in a valid value', () => {
253+
const result = parseEnv([
254+
'KEY_WITH_COMMENT_IN_VALUE="value # this is a comment"',
255+
].join('\n'));
256+
257+
assert.deepStrictEqual(result, {
258+
KEY_WITH_COMMENT_IN_VALUE: 'value # this is a comment',
259+
});
260+
});
203261
});

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL