tonerdo / dotnet-env

A .NET library to load environment variables from .env files
MIT License
427 stars 50 forks source link

Fails to parse when environment file has dash (-) #59

Closed jrsearles closed 3 years ago

jrsearles commented 3 years ago

I want to thank you for this library. It seems to be the most complete dotnet libraries out there supporting env file parsing. I did run into one issue regarding environment variables with a dash in them:

Example file:

test-dash=value

Result: Sprache.ParseException : Parsing failure: unexpected '-'; expected = (Line 1, Column 5); recently consumed: test

This is a perfectly legal environment variable name. In fact, it appears that both Windows & Linux support all characters but equals character (=) and NUL in environment variable names. It looks like this library is basically supporting the pattern "[a-zA-Z]+[a-zA-Z0-9]*". I'm wondering if you'd be open to at least allowing the dash? I'd be willing to contribute a PR - it seems like it'd be easy to support.

rogusdev commented 3 years ago

Thanks for your appreciation! :) Where do you see "supports all characters but equals" in docs? The regex for identifiers that I use is the official set: https://stackoverflow.com/questions/2821043/allowed-characters-in-linux-environment-variable-names

jrsearles commented 3 years ago

The first answer there is what i am referring to with Linux.

These strings have the form name=value; names shall not contain the character '='. For values to be portable across systems conforming to IEEE Std 1003.1-2001, the value shall be composed of characters from the portable character set (except NUL and as indicated below).

For Windows, documentation can be found here implicitly.

The name of an environment variable cannot include an equal sign (=).

I looked at a couple other implementations.

I haven't found a spec for envfiles (i don't think one exists), so it looks like different implementations are taking different approaches.

jrsearles commented 3 years ago

Couple more:

rogusdev commented 3 years ago

All of the functionality I have implemented is explicitly based on what is actually possible in a linux (bash/zsh) shell. To my knowledge, per the rules mentioned in the stack overflow above, there is no way to have an env var that has a hyphen in it.

I strongly, strongly encourage you to try it yourself -- these are what I get, and therefore I will not support them in dotenv files since you cannot have env vars named this way in a shell:

bash-3.2$ export test-dash=value
bash: export: `test-dash=value': not a valid identifier
(zsh) ~ % export test-dash=value
export: not valid in this context: test-dash

Also, there is no schema/spec for env files, they are ad hoc at this point. My system has been to match, as closely as possible, what is actually possible with real env vars. Other libraries have been more liberal in their interpretations.

jrsearles commented 3 years ago

Yeah - i agree that the shell has this limitation, but environment variables themselves do not. The same stack overflow answer does reference this:

Other characters may be permitted by an implementation; applications shall tolerate the presence of such names.

rogusdev commented 3 years ago

So, serious question: how would you set these env vars to be used by your application in production? Remember that, as noted in the README, you should not be using a .env file in production. You should be actually setting env vars in the environment. So, the support from the shell is a critical part of this and allowing things in the localdev file that are impossible to do in production strikes me as leading people down a very bad path.

Which is to say, if you have some other approach for production that makes it so that dashes can be present in env vars in production, I definitely want to consider that case!

jrsearles commented 3 years ago

Well, we don't use env files in production. I believe they use Puppet profiles and/or PowerShell scripts. I'm not too versed in Puppet but i've confirmed that this is easily achievable in PowerShell:

PS > [System.Environment]::SetEnvironmentVariable("test-dash", "value")
PS > [System.Environment]::GetEnvironmentVariable("test-dash")
value

These environment variables are already in use for us. I'm just trying to update some of our local tooling.

rogusdev commented 3 years ago

Got it. If you are using it in production already, I'm ok with supporting that.

rogusdev commented 3 years ago

@jrsearles This is published now, enjoy! https://www.nuget.org/packages/DotNetEnv/2.2.0