Skip to content
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

Won't read from variable when it contains $ character #52

Closed
dkeza opened this issue Feb 21, 2018 · 10 comments · Fixed by #58
Closed

Won't read from variable when it contains $ character #52

dkeza opened this issue Feb 21, 2018 · 10 comments · Fixed by #58

Comments

@dkeza
Copy link

dkeza commented Feb 21, 2018

go version go1.9.2 windows/386

When I am reading from variable in my .env file which has $ in string:
MAIL_SECRET=xTvDqw$27

os.Getenv("MAIL_SECRET")
gives xTvDqw7 back, ignoring $2

why is that, have $ sign some special meaning?

@gcstang
Copy link

gcstang commented Mar 13, 2018

I have the same exact issue
go version 1.10 mac High Sierra

It has to do with this function being used which looks like it's meant to replace environment settings but it replaces dollar signs.

Line 295 to 300 in godotenv value = os.Expand(value, func(key string) string { if val, ok := envMap[key]; ok { return val } return "" })

@gcstang
Copy link

gcstang commented Mar 14, 2018

@dkeza I changed over to this project and it works correctly (plus it's active this one seems dead)
go get github.com/ufoscout/go-up

@joho
Copy link
Owner

joho commented Mar 14, 2018

Hi there, sorry I hadn't replied.

Project's not dead, I'm just going through a very hectic phase of life right now. I will look into this, I'm suspecting a regression in some of the more recent updates to variable substitution.

@gcstang
Copy link

gcstang commented Mar 14, 2018

@joho no worries, it's understandable.
Thank you for looking into the issue.

@egorse
Copy link
Contributor

egorse commented Mar 31, 2018

Couple of notes - os.Expand does not supports escaping (so no workarround for this issue);
godotenv expand does not takes real env - so, for instance, can not expand PWD w/o defined in file :S This is more question - is that desired or related bug?

lucastetreault added a commit to lucastetreault/godotenv that referenced this issue Sep 4, 2018
@lucastetreault
Copy link
Contributor

Hey @egorse, I just stumbled upon the same issue and created the following pull request: #57

I think that's a decent compromise...

@joho
Copy link
Owner

joho commented Sep 5, 2018

There's definitely a bug here, maybe more than one, but not the one I think people are trying to fix.

One goal of the library (that the library is frequently falling short of) is to have cross compatibility with the rails and node dotenv file formats. So to decide what the right fix is I went and tested the same config against the ruby version https://gist.github.com/joho/e5ce5415f75feac3c7dedc8ea38611cb

So the expansion in the Go version is definitely wrong, but #57 makes it wronger.

@lucastetreault
Copy link
Contributor

Lol ok, then I guess I'll stick to my fork that allows my passwords to have $'s in them. I don't know anything about the Ruby dotenv but the node dotenv package actually doesn't do variable expansion at all:

https://www.npmjs.com/package/dotenv#what-about-variable-expansion

@joho
Copy link
Owner

joho commented Sep 5, 2018

https://github.com/bkeepers/dotenv/blob/master/spec/dotenv/parser_spec.rb#L49-L77

The ruby version is the original, spun out of heroku who popularised the 12 factor approach (for better or worse), and this and the node version are both clones, so I treat ruby as canonical.

The bug that does need to be fixed and will allow you to have dollar signs in passwords is that \$ isn't escaping correctly. MAIL_SECRET=xTvDqw\$27 should map to "xTvDqw$27" when you call os.Getenv (but it doesn't). I'll have a quick look now and see if I can get that going again.

@joho
Copy link
Owner

joho commented Sep 5, 2018

Ah, as @egorse points out os.Expand doesn't support escaping and just generally appears to have made some implementation tradeoffs out of step with other languages (see golang/go#24345)

I guess to get escaping working properly I'll need my own expansion logic.

lucastetreault added a commit to lucastetreault/godotenv that referenced this issue Sep 11, 2018
…Expand

Copy over the tests from https://github.com/bkeepers/dotenv/blob/master/spec/dotenv/parser_spec.rb
related to expanding variables and implement the required changes. I also realized as part of this
that this implementation was not handling values in single quotes properly (e.g.: not the same was
as the ruby package mentionned) so that has been fixed as well along with the related tests.

Fixes: joho#52
@joho joho closed this as completed in #58 Sep 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants