Blank environment variables aren't loaded (even in a POSIX environment) about dotenv-hs

[habibalamin]

I've fixed this upstream, and it seems to be slated for the 8.4.1 release, which looks like it's going to be released on the 18th January 2018 12am (~6 months from now).

[habibalamin]

As it happens, @CristhianMotoche did some tests with the major Ruby & JS implementations and they both set blank variables. I'm not sure how those implementations would behave on Windows.

I also noted on #7 that there is an open GHC bug ticket claiming that Windows does, in fact, support blank environment variables.

From what I can gather from a quick Google, most sources corroborate the original claim of Windows not supporting blank environment variables. This could be because:

1. Repetition of the claim has made others believe it to be the truth and repeat it themselves.
2. The official Windows documentation linked to that suggests — and it does only suggest — that blank environment variables are supported is misleading.

[habibalamin]

Wowee, what a journey.

[habibalamin]

I shut down the machine, and it's now updating 🙄, but I'll test that once that's done.

Yes, the default setEnv is defined to not be able to set blank environment variables for consistency. The unix package (which contains System.Posix.setEnv), as expected, won't build on the Windows system without something like Cygwin; I haven't tried with Cygwin, but I'll try that too.

We could also define our own setEnv that allows blank environment variables on both Windows and POSIX, but that sounds like perhaps the effort/readability/maintainability burden isn't worth it, although I wouldn't know (we'd probably have to interact with the FFI).

[habibalamin]

Okay, Ruby 2.3.3's Kernel#exec inherits the environment:

Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>irb
irb(main):001:0> ENV['TEST'] = 'test'
=> "test"
irb(main):002:0> exec 'cmd'
Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>echo %TEST%
test

I had the same result as in C here. Setting a blank environment variable is retrievable through the language, but when spawning a cmd.exe process, it treats blank environment variables as unset:

Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>irb
irb(main):001:0> ENV['TEST']
=> nil
irb(main):002:0> ENV['TEST'] = ''
=> ""
irb(main):003:0> ENV['TEST']
=> ""
irb(main):004:0> exec 'cmd'
Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>echo %TEST%
%TEST%

Perhaps Ruby or Windows' C API just store blank environment variables in a special place outside of the actual environment and that's why the shell doesn't recognise it; I tested that by execing two programs I controlled.

First, I modified the C program to this:

#include <stdlib.h>
#include <Windows.h>
#include <stdio.h>
#include <process.h>

int main(int argc, char *argv[])
{
  char test[50];

  SetEnvironmentVariable("TEST", "");
  GetEnvironmentVariable("TEST", test, 51);
  printf("TEST: '%s'\n", test);

  _execlp("ruby", "ruby",  "enver.rb", NULL);

  printf("exec failed!\n");
  return 0;
}

and created a Ruby program like this

puts "TEST: #{ENV['TEST'].inspect}"

The output is as you would expect:

Microsoft Windows [Version 6.1.7601]
Copyright (c) 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>.\enver
TEST: ''

C:\Users\IEUser\Documents>TEST: ""

Some timing issues, but it makes the point. Of course, when passing NULL to SetEnvironmentVariable, I get:

Microsoft Windows [Version 6.1.7601]
Copyright (c) 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>.\enver
TEST: 'íéh▀■   ┌ÿ►v·á◄v░♫''

C:\Users\IEUser\Documents>TEST: nil

Then I did it the other way around:

puts "TEST: #{ENV['TEST'].inspect}"
exec '.\enver'

#include <stdlib.h>
#include <Windows.h>
#include <stdio.h>
#include <process.h>

int main(int argc, char *argv[])
{
  char test[50];

  GetEnvironmentVariable("TEST", test, 51);
  printf("TEST: '%s'\n", test);

  return 0;
}

Here's the output of that:

Microsoft Windows [Version 6.1.7601]
Copyright (c) 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>ruby enver.rb
TEST: nil
TEST: '╨d╡4■   ┌ÿ►v·á◄v└♫X'

C:\Users\IEUser\Documents>set TEST=test

C:\Users\IEUser\Documents>ruby enver.rb
TEST: "test"
TEST: 'test'

C:\Users\IEUser\Documents>

I think this is good evidence that it's being stored in the actual environment, as it's being transferred cross-language as we would expect.

Here's what I got for the dotenv gem with your .env example in #44:

Microsoft Windows [Version 6.1.7601]
Copyright (c) 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>ruby dotenv.rb
Foo is set
FOO: ""
BAR: "1234"

C:\Users\IEUser\Documents>

Although I slightly modified the program (Rubyists use string interpolation, which automatically calls #to_s on the value; string1 + nil + string2 explodes, but "prefix#{nil}suffix" evaluates to "prefixsuffix"):

require 'dotenv'

Dotenv.load('.env')

if ENV['FOO'].nil?
  puts 'Foo is not set'
else
  puts 'Foo is set'
end

# fun fact: #inspect is Ruby's equivalent of Haskell's `show`;
# every object has it by default, except when inheriting from
# BasicObject.
puts "FOO: #{ENV['FOO'].inspect}"
puts "BAR: #{ENV['BAR'].inspect}"

I used your example for node and I actually was not expecting this:

Microsoft Windows [Version 6.1.7601]
Copyright (c) 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>node dotenv.js
Foo is not set
Foo
Bar 1234

C:\Users\IEUser\Documents>

That looks like everything.

[CristhianMotoche]

I'll let this issue open, because this is a problem that we shouldn't fix here. However, if you want to have empty env vars you could use something like the code below in your configuration.

fromMaybe "" <$> lookupEnv "ENV_VAR"

For instance, if ENV_VAR is empty or not set it will be "" in your configuration, otherwise it will be any value you have in ENV_VAR.

[habibalamin]

You can track the progress of this bug upstream (it's technically a GHC bug).

[habibalamin]

So now that the latest version of base, 4.11.0.0, that was uploaded to Hackage on Monday, includes System.Environment.Blank, which works with Windows and POSIX, I think we ought to look back into this.

I know there are backward-compatibility (and forward-compatibility) concerns. Eventually, System.Environment.Blank will be folded into System.Environment in the next major GHC version (IIRC).

[CristhianMotoche]

@habibalamin I just pushed 0.8.0 to hackage.
I suppose we can close this issue now. Thanks a lot for your contribution. 😁

[jsl]

Thanks @habibalamin! I'm leaning towards having an option as well to allow blank values. Maybe the default is to do whatever works on Linux / Mac and Windows users need to specify an option? I wish we could find precedent in another dotenv lib that we could use for inspiration. Feel free to post here if you have observed, or have time to research behavior in other dotenv libs (Node, Python, Ruby etc) that you like or dislike.

[habibalamin]

I used this program to test the documentation:

#include <stdlib.h>
#include <Windows.h>
#include <stdio.h>
#include <process.h>

int main(int argc, char *argv[])
{
  char test[50];

  SetEnvironmentVariable("TEST", "");
  GetEnvironmentVariable("TEST", test, 51);
  printf("TEST: '%s'\n", test);

  // searches PATH, inherits environment
  _execlp("cmd", "cmd", NULL);

  printf("exec failed!\n");
  return 0;
}

I tested it on a Windows 7 VM:

Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
%TEST%

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
%TEST%

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
%TEST%

C:\Users\IEUser\Documents>

Perhaps more interestingly:

Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>set TEST=test

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
test

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
test

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
%TEST%

C:\Users\IEUser\Documents>

If the environment variable is set, it seems to be forgotten on the third — consistently the third — nested exec call, but it doesn't unset it the first time it's set to the empty string. The documentation suggests that it would be unset if it were given an argument of NULL, but it didn't say anything about giving it an argument of a blank string, so that's technically valid.

Incidentally, I did also test SetEnvironmentVariable with a null value and that doesn't forget either, so the documentation seems to be inapplicable for whatever reason (it's possibly referencing Windows 8).

The most interesting finding is that when calling GetEnvironmentVariable, everything behaved as expected; blank environment variables were actually printed as blank (well, not printed), and when set to NULL, printing the environment variable results in funny characters being printed.

It could be that Windows does support blank environment variables under the hood, but the cmd.exe shell itself simply treats those variables as unset. In that case, it seems perfectly fine to use in our own programs, but I'm speculating.

[habibalamin]

I'm going to be removing the VM, by the way, so if there's any particular tests you want me to do, you have 4 hours.

[CristhianMotoche]

Thanks for the research @habibalamin. I think the problem we have is that we cannot set env vars in POSIX environments. That is because the way setEnv in System.Environment is defined. Right?

Could you please test other dotenv libraries from other languages (JS, Ruby, or Python) and see what is their behaviour? You could use the examples that I did

[CristhianMotoche]

Another solution could be using CPP like in #7. However, I'm not sure if we want to use CPP for that. Let me know what you think about it?

[CristhianMotoche]

@habibalamin Amazing job! Thanks a lot for your research. Now we understand in a better way the behaviour of Windows with empty env vars.

Well, now, for the matter of having blank env vars. We can make a list of the possible solutions and then choose one, or in the other hand not allow empty env vars.

[CristhianMotoche]

Feel free to add your solution:

• Create an FFI that works in both OS. (Proposed by @habibalamin)
• Using CPP to load System.Posix.setEnv when the OS is not Windows.

[habibalamin]

Quoting from my other comment:

I realise the danger of adding complexity through feature toggles, but allowing the user to pick their poison seems like the right option for situations where neither option is entirely without issue. Maybe a --unix/--allow-empty-values flag [would be useful].

In this case, this is another step we might want to take if we go for the blank environment variables only on POSIX option. This would allow the users to decide whether they want a consistent experience on all platforms (good for teams) or improved semantics for their preferred platform (good for homogenous teams/individuals).

The FFI option is probably going to take longer, but I will take a quick look at that and see what I can do.

[habibalamin]

@CristhianMotoche if you look at the PR, you can see I had trouble with setting blank environment variables across the FFI, despite the fact that I can do it directly in C and the Ruby API allows it.

I thought it might have something to do with sending CString values across the FFI, but I also created a dedicated function with no arguments that itself calls SetEnvironmentVariable with a blank string directly in C, and that didn't work either.

I'd like it if someone could take a look at the PR, see if I missed anything, but I don't have high hopes. This problem could be why the GHC folks thought Windows doesn't support blank variables too, and Node doesn't seem to allow it, so they either didn't want to or had issues, too.

I could take a look at the Ruby project C source and see how they do it, but our best option is looking like support for UNIX only (possibly with a flag, so that the default is consistency across platforms).

[habibalamin]

The way Ruby does it is very complex. Obviously, there's some extra work for platforms like Solaris there, but it seems unwieldy and like something we don't want to adopt for something that should be so basic.

I'm not sure how it works, and it would probably take me hours or days to figure it out (I can get by in C, but I don't really know it or use it).

So, I propose we just ignore Windows support for this. Ideally, we'd default to consistency on all platforms, and use a flag to enable the feature for teams who exclusively use Unix platforms.

[CristhianMotoche]

Closing this PR because of #51 (comment)

[CristhianMotoche]

😮 Great! Thanks for the news @habibalamin 😄

[CristhianMotoche]

Awesome. Thanks a lot @habibalamin