Logic error in GetEnv()

[UlrichEckhardt]

Description of the bug:

If you force Bazel to use an empty PATH environment variable (e.g. build --action_env=PATH= in .bazelrc) and run tests, you will get an error:

ERROR(tools/test/windows/tw.cc:358) value: 0 (0x00000000), arg: PATH: Failed to read envvar

Looking at the implementation, it uses GetEnvironmentVariableW, which has following possible results:

• The var was not defined.
• The var was defined and fits into the buffer.
• The var was defined and does not fit into the buffer.
• Some error happened.

The bug is in the handling of a zero returnvalue, which can mean three things:

• The var is undefined. In that case, GetLastError() == ERROR_ENVVAR_NOT_FOUND.
• The var is defined and has zero length. In that case, GetLastError() == 0. This is the case that is not handled correctly in the code.
• Some error happened. GetLastError() gives you further info on the error.

Suggested fix for the condition:

-  if (size == 0 && err == ERROR_ENVVAR_NOT_FOUND) {
+  if (size == 0 && ((err == 0) || (err == ERROR_ENVVAR_NOT_FOUND))) {

...plus of course extending the existing unit tests to permanently ban that bug from the codebase.

Cheers & happy hacking!

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

As mentioned above, it's the PATH setting plus running tests (C++ or C test code based on Google Test). I haven't extracted a proper minimal example yet. I would like to avoid that but I can, if above info doesn't suffice.

Which operating system are you running Bazel on?

MS Windows

What is the output of bazel info release?

5.1.1

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

n/a

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No similar issues were reported yet.

Any other information, logs, or outputs that you want to share?

There is a workaround: Don't use an empty PATH. In order to get an effectively empty path, you can set it to ; (single semicolon) instead.

[meteorcloudy]

@UlrichEckhardt Thanks for reporting this bug, would you mind sending a PR to fix this issue?

[UlrichEckhardt]

Can you fix it without that? I have no insight into the Bazel codebase and I wouldn't know how to build and test it myself.

[meteorcloudy]

You can follow https://bazel.build/contribute/getting-started ;)

[UlrichEckhardt]

Thanks for the offer, but I think I'll pass. Doing this properly with according tests would take way more time than I would like to invest here.