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

Passing empty environment variables to child processes convert to 'undefined' using ConEmu + Node #1209

Open
nicojs opened this Issue Aug 1, 2017 · 14 comments

Comments

Projects
None yet
3 participants
@nicojs

nicojs commented Aug 1, 2017

Versions

ConEmu build: 170402 x64
OS version: Windows 10 version 1703 x64
Used shell version git-bash: v2.11.0

Problem description

When executing a process which spawns a child process, which in turn creates another child process, the empty environment variables are passed through as 'undefined'.

Steps to reproduce

  1. Install the latest version of node&npm: http://nodejs.org/
  2. Clone this repo git clone git@github.com:nicojs/reproduce-conemu-child-process-environment-bug.git
  3. run npm install.
  4. run npm test

Actual results

When ran from within ConEmu: the test failes

1) the env should contain "npm_config_onload_script":
     Error: "npm_config_onload_script": "undefined"!"

The test also prints the environment variables to screen. You can see a lot of "undefined" values (the string literal, not an absent value).

For example: "npm_config_onload_script": "undefined",

Expected results

If ran from cmd or git-bash using mintty directly: the test passes

√ should contain "npm_config_onload_script"

If you now look at the environment variables on screen, you don't see the "undefined" values. Just empty strings.

For example: "npm_config_onload_script": "",

Additional files

Settings,
git-bash-mitty
using-con-emu

@Maximus5

This comment has been minimized.

Show comment
Hide comment
@Maximus5

Maximus5 Aug 1, 2017

Owner

If ran from cmd or git-bash using mintty

THIS is NOT fair comparison. ConEmu doesn't ruin node. From your screenshots I can make a conclusion you run bash. Report this to your bash bundle authors.
Third-party application problems

Owner

Maximus5 commented Aug 1, 2017

If ran from cmd or git-bash using mintty

THIS is NOT fair comparison. ConEmu doesn't ruin node. From your screenshots I can make a conclusion you run bash. Report this to your bash bundle authors.
Third-party application problems

@Maximus5

This comment has been minimized.

Show comment
Hide comment
@Maximus5

Maximus5 Aug 1, 2017

Owner

Also you said: it runs from cmd.
So run cmd.exe in ConEmu (as shell) and check!

Owner

Maximus5 commented Aug 1, 2017

Also you said: it runs from cmd.
So run cmd.exe in ConEmu (as shell) and check!

@nicojs

This comment has been minimized.

Show comment
Hide comment
@nicojs

nicojs Aug 1, 2017

The same issue when running cmd.exe in ConEmu.

using-con-emu-cmd

But not using command prompt:

using-command-promt-cmd

The only way for me to reproduce this issue is to run it in ConEmu with any shell (cmd or git bash).

I read trough the Third-party application problems page before submitting this issue, but seeing i can only reproduce it in ConEmu, i decided to report it here.

nicojs commented Aug 1, 2017

The same issue when running cmd.exe in ConEmu.

using-con-emu-cmd

But not using command prompt:

using-command-promt-cmd

The only way for me to reproduce this issue is to run it in ConEmu with any shell (cmd or git bash).

I read trough the Third-party application problems page before submitting this issue, but seeing i can only reproduce it in ConEmu, i decided to report it here.

@Maximus5

This comment has been minimized.

Show comment
Hide comment
@Maximus5

Maximus5 Aug 1, 2017

Owner

Just checked. And...

2017-08-01_20-13-56

It is working with node 7.9.0. Regression in node?

Owner

Maximus5 commented Aug 1, 2017

Just checked. And...

2017-08-01_20-13-56

It is working with node 7.9.0. Regression in node?

@Maximus5

This comment has been minimized.

Show comment
Hide comment
@Maximus5

Maximus5 Aug 1, 2017

Owner

ConEmu does not change environment variables while running applications.

https://conemu.github.io/en/WindowsEnvironment.html

Owner

Maximus5 commented Aug 1, 2017

ConEmu does not change environment variables while running applications.

https://conemu.github.io/en/WindowsEnvironment.html

@nicojs

This comment has been minimized.

Show comment
Hide comment
@nicojs

nicojs Aug 2, 2017

It is working with node 7.9.0. Regression in node?

I think so yeah. I just tried it with node 7.9, works fine. Switching to node 8.0.0 seems to break it. I will open an issue at node.

nicojs commented Aug 2, 2017

It is working with node 7.9.0. Regression in node?

I think so yeah. I just tried it with node 7.9, works fine. Switching to node 8.0.0 seems to break it. I will open an issue at node.

@nicojs

This comment has been minimized.

Show comment
Hide comment
@nicojs

nicojs Aug 2, 2017

I'll close this issue for now, hope we can find out whats happening at Node side. Thanks for your help @Maximus5

nicojs commented Aug 2, 2017

I'll close this issue for now, hope we can find out whats happening at Node side. Thanks for your help @Maximus5

@nicojs nicojs closed this Aug 2, 2017

@nicojs

This comment has been minimized.

Show comment
Hide comment
@nicojs

nicojs Aug 2, 2017

@Maximus5 it appears to be caused by an injected dll at ConEmu side: "ConEmuHq.dll"

nodejs/node#14593 (comment)

People are able to reproduce it in multiple node versions. Any ideas?

nicojs commented Aug 2, 2017

@Maximus5 it appears to be caused by an injected dll at ConEmu side: "ConEmuHq.dll"

nodejs/node#14593 (comment)

People are able to reproduce it in multiple node versions. Any ideas?

@nicojs nicojs reopened this Aug 2, 2017

@refack

This comment has been minimized.

Show comment
Hide comment
@refack

refack Aug 2, 2017

@Maximus5 IMHO the edge case here is that in node we can set an empty string env var:

process.env.foobar='';

attached is a minimally reproducing script and a Procmon PML trace
conemu-1209.zip
The env var foobar get converted from an empty string to a NULL with the first spawn
In the second spawn a bug on node's side converts the js undefined value to its string representation

refack commented Aug 2, 2017

@Maximus5 IMHO the edge case here is that in node we can set an empty string env var:

process.env.foobar='';

attached is a minimally reproducing script and a Procmon PML trace
conemu-1209.zip
The env var foobar get converted from an empty string to a NULL with the first spawn
In the second spawn a bug on node's side converts the js undefined value to its string representation

@nicojs

This comment has been minimized.

Show comment
Hide comment
@nicojs

nicojs Aug 2, 2017

@refack so this is an issue on node side only? Or on ConEmu side as well?

nicojs commented Aug 2, 2017

@refack so this is an issue on node side only? Or on ConEmu side as well?

@refack

This comment has been minimized.

Show comment
Hide comment
@refack

refack Aug 2, 2017

AFAICT on both sides
ComEmu converts empty strings to null
node convert null to undefined

refack commented Aug 2, 2017

AFAICT on both sides
ComEmu converts empty strings to null
node convert null to undefined

Maximus5 added a commit that referenced this issue Aug 3, 2017

gh-1209: Node and 'undefined' in the `process.env.foobar`.
  Seems like Node checks for non-zero GetLastError()
  instead of ERROR_ENVVAR_NOT_FOUND after GetEnvironmentVariable call.
@Maximus5

This comment has been minimized.

Show comment
Hide comment
@Maximus5

Maximus5 Aug 3, 2017

Owner

ConEmu doesn't change environment. And hooked GetEnvironmentVariable properly returns empty string.
But Node checks for non-zero GetLastError() instead of described ERROR_ENVVAR_NOT_FOUND.

Owner

Maximus5 commented Aug 3, 2017

ConEmu doesn't change environment. And hooked GetEnvironmentVariable properly returns empty string.
But Node checks for non-zero GetLastError() instead of described ERROR_ENVVAR_NOT_FOUND.

@nicojs

This comment has been minimized.

Show comment
Hide comment
@nicojs

nicojs Aug 4, 2017

What does the ConEmuHq.dll do? Because it does change the behavior of node (regardless of the node bug)

nicojs commented Aug 4, 2017

What does the ConEmuHq.dll do? Because it does change the behavior of node (regardless of the node bug)

@Maximus5

This comment has been minimized.

Show comment
Hide comment
@Maximus5

Maximus5 Aug 4, 2017

Owner

It's all in docs: http://conemu.github.io/en/ConEmuHk.html

TL;DR - it's a required component to bypass conhost bugs and limitations.

Owner

Maximus5 commented Aug 4, 2017

It's all in docs: http://conemu.github.io/en/ConEmuHk.html

TL;DR - it's a required component to bypass conhost bugs and limitations.

@refack refack referenced this issue Aug 5, 2017

Closed

process: EnvGetter to use libuv #14641

2 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment