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
Fixing a bug where empty variables in .env files were read as multi-line values (#3934) #3951
Conversation
src/test/functions/env.spec.ts
Outdated
// { | ||
// description: "should ignore comments", | ||
// input: ` | ||
// FOO=foo # comment | ||
// # line comment 1 | ||
// # line comment 2 | ||
// BAR=bar # another comment | ||
// `, | ||
// want: { FOO: "foo", BAR: "bar" }, | ||
// }, | ||
// { | ||
// description: "should ignore empty lines", | ||
// input: ` | ||
// FOO=foo | ||
|
||
// BAR=bar | ||
|
||
// `, | ||
// want: { FOO: "foo", BAR: "bar" }, | ||
// }, |
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.
Do we want to uncomment this?
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.
🤦 Forgot to save the file before commiting
src/functions/env.ts
Outdated
@@ -49,7 +49,7 @@ const LINE_RE = new RegExp( | |||
"(" + // begin optional value | |||
"\\s*'(?:\\\\'|[^'])*'|" + // single quoted or | |||
'\\s*"(?:\\\\"|[^"])*"|' + // double quoted or | |||
"[^#\\r\\n]+" + // unquoted | |||
"[^#=\\r\\n]*" + // unquoted |
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.
q: Did we need =
?
IIUC, we are making something like
FOO=bar=a
from being a valid entry ({FOO: "bar=a"}
) to an invalid one.
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.
Hmmm, its a bit of a weird case - i guess what we really want to do is allow =, but not after a line break?
So something like
[[^#\\r\\n][^#=\\r\\n]]
There may be other regex tricks that could solve this as well - maybe replacing the \\s
that. matches whitespace after the equals with something that doesn't match newlines?
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.
Ahhhh, i just read your original code comments more closely, and realized we explicitly don't support multiline unquoted values. I think i know how to fix this, will play around a bit and make sure all the tests work.
Description
Fixes an issue where the .env parser would mishandled empty values. The following .env file:
would be parsed into
Fixes #3934
Scenarios Tested
Added a test case to cover this