Repository navigation
--env-file does not support inner quotes (does not behave like dotenv) #54134
Description
Activity
Pull requests are welcome
Pull requests are welcome
Hi, this is my first time contributing to Node.js, I will take this issue as my first contribution. I think this is a good starting place.
Reacted by Clemens Stolle- addeddotenvIssues and PRs related to .env file parsing.Issues and PRs related to .env file parsing.
on Aug 1, 2024 - addedconfirmed-bugIssues and PRs for confirmed bugs.Issues and PRs for confirmed bugs.
on Aug 1, 2024 added additional information around dotenv and the implications of the bug
For reference, here are parsing rules from dotenv project page:
What rules does the parsing engine follow?
The parsing engine currently supports the following rules:
BASIC=basicbecomes{BASIC: 'basic'}- empty lines are skipped
- lines beginning with
#are treated as comments #marks the beginning of a comment (unless when the value is wrapped in quotes)- empty values become empty strings (
EMPTY=becomes{EMPTY: ''}) - inner quotes are maintained (think JSON) (
JSON={"foo": "bar"}becomes{JSON:"{\"foo\": \"bar\"}") - whitespace is removed from both ends of unquoted values (see more on
trim) (FOO= some valuebecomes{FOO: 'some value'}) - single and double quoted values are escaped (
SINGLE_QUOTE='quoted'becomes{SINGLE_QUOTE: "quoted"}) - single and double quoted values maintain whitespace from both ends (
FOO=" some value "becomes{FOO: ' some value '}) - double quoted values expand new lines (
MULTILINE="new\nline"becomes
source: https://github.com/motdotla/dotenv?tab=readme-ov-file#what-rules-does-the-parsing-engine-follow
Reacted by Luboš MatejčíkThere is one more invalid use case:
MP_#CRAZY_COMMENT="2: foo bar\ni am "on" newl\nine, 'yo'"is parsed into env by Node, but dotenv skips it.
@anonrig @macrozone How strict do we want to be with dotenv compatibility? I've got a working fix that is handling all tests of dotenv. It improves compatibility and simplifies parser, but behaves differently with multiline "" values that have unbalanced ".
Covering all edge-cases of dotenv without using their regexp is pretty crazy.
Covering all edge-cases of dotenv without using their regexp is pretty crazy.
Agreed. We started following dotenv through their tests, but I think it's ok to diverge from there for extreme edge cases. I don't think we need to be 100% compliant with dotenv.
@anonrig @macrozone How strict do we want to be with dotenv compatibility? I've got a working fix that is handling all tests of dotenv. It improves compatibility and simplifies parser, but behaves differently with multiline "" values that have unbalanced ".
Covering all edge-cases of dotenv without using their regexp is pretty crazy.
I actually personally don't care about the compatiblitiy, but at the moment some env var values are absolutly impossible to declare. Namly one that contains: a line break, a double quote a backtick and a single quote. There is no way to declare such a env var.
Allowing to escape quotes would also solve it, but that was attempted and rejected because "its not compatible with dotenv".
I am also fine when it breaks with actual line breaks, since thats also bugged in dotenv. When using quotes you can encode line breaks with
\n.So if this works, it would be fine for me:
MY_VAR="singlequote: ', double quote: ", a line break: \n(i am on newline) and a backtick: `. that is all i need"I agree that "a line break, a double quote, and a single quote" should be supported, and it is considered as a bug.
Reacted by Toni VillenaI agree that "a line break, a double quote, and a single quote" should be supported, and it is considered as a bug.
don't forget the backtick 😁 (woops, i forgot also to mentione it above)
Just tried your example:
.env file:
MP_MY_VAR="singlequote: ', double quote: ", a line break: \n(i am on newline) and a backtick: `. that is all i need"dotenv:
MP_MY_VAR=singlequote: ', double quote: ", a line break: (i am on newline) and a backtick: `. that is all i needmy changes:
MP_MY_VAR=singlequote: ', double quote: ", a line break: (i am on newline) and a backtick: `. that is all i needSo it looks like it will handle it exactly like dotenv.
I already had to make it much more complex than needed to handle all other edge cases.
It would be such a simple parser if we only had to look for balanced double-quotes and newlines.don't forget the backtick 😁 (woops, i forgot also to mentione it above)
Now we are getting away from reality, lol. What's the usecase/example of an environment variable that contains all of these characters?
Reacted by Marco Wettstein@macrozone Added your case to tests and opened a PR: #54215
Reacted by Marco WettsteinReacted by Toni VillenaNow we are getting away from reality, lol. What's the usecase/example of an environment variable that contains all of these characters?
It should not be node's decision what is an allowed env var value and what not. An env var value is a string and any string should be somehow be encodeable in a .env file.
In my case I am writing tooling that creates those .env files on the fly in a ci/cd pipeline from another store. Those can be any strings and Its hard to mirror arbitrary decisions what are valid strings and what not. This is how I noticed those problems in the first place.
luckily fixing the inner quotes problem solves the issue.
(also in bash its no problem to declare such a variable thanks to escaping
MY_ENV_VAR="singlequote: ', double quote: \", a line break: (i am on newline) and a backtick: \`. that is all i need" node envtest.js)
@macrozone Added your case to tests and opened a PR: #54215
thank you so much for your effort!
Can you also bring back the inline comments?
MY_VAR=dfadfad09845*?)!='^=++!+^. #inline commentReacted by Code Scratcher@marekpiechut It also skips lines that start with
;, which is standard for INI files.Reacted by Emrah AtilkanPlease assign me this issue. I want to fix this
Reacted by Emrah AtilkanHey, I'd like to fix this bug. Kindly assign this to me.
You're free to open a pull-request.
Version
v20.16.0
Platform
Subsystem
No response
What steps will reproduce the bug?
inspired by this comment: #50814 (comment)
create an
.envfile:test with dotenv:
$ node envtest-dotenv.jstest with
--env-file$ node --env-file=.env envtest.jsHow often does it reproduce? Is there a required condition?
always
What is the expected behavior? Why is that the expected behavior?
outputs should be the same.
What do you see instead?
output are different, node native terminates at the first occurrence of the double quote:
compare that to dotenv:
Additional information
--env-file#50814And