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

Cannot delete property from process.env #27990

Open
iamogbz opened this issue May 31, 2019 · 7 comments

Comments

Projects
None yet
5 participants
@iamogbz
Copy link

commented May 31, 2019

  • Version: 12.3.1
  • Platform: Darwin Kernel Version 18.6.0: Thu Apr 25 23:16:27 PDT 2019; root:xnu-4903.261.4~2/RELEASE_X86_64 x86_64
  • Subsystem: process

Configuring a property on the process.env object and deleting it results in weird behaviour.

Documented in gist//60e654342532f80ae264128909e84198

curl -s https://gist.githubusercontent.com/iamogbz/60e654342532f80ae264128909e84198/raw/9329e2a1595b3b365d43ca5e59f242251df56f96/process.env.test.js | node

Seems like once a property has been configured with a getter setter, then the property can never be truly deleted from the process.env object

@Himself65

This comment has been minimized.

Copy link
Contributor

commented May 31, 2019

output on node v10.15

>>> plain object <<<
prop is not defined
value undefined
hasProperty undefinedProp false
ownProperty undefinedProp false

prop is declared as 1 with assignment (=)
value 1
hasProperty undefinedProp true
ownProperty undefinedProp true

prop is deleted true
value undefined
hasProperty undefinedProp false
ownProperty undefinedProp false

prop is configured as 1 with value
value 1
hasProperty undefinedProp true
ownProperty undefinedProp true

prop is deleted true
value undefined
hasProperty undefinedProp false
ownProperty undefinedProp false

prop is configured as 1 with getter|setter
value 1
hasProperty undefinedProp true
ownProperty undefinedProp true

prop is deleted true
value undefined
hasProperty undefinedProp false
ownProperty undefinedProp false

prop is redeclared as 1 with assignment (=)
value 1
hasProperty undefinedProp true
ownProperty undefinedProp true

prop is deleted true
value undefined
hasProperty undefinedProp false
ownProperty undefinedProp false


>>> process.env <<<
prop is not defined
value undefined
hasProperty undefinedProp false
ownProperty undefinedProp false

prop is declared as 1 with assignment (=)
value "1"
hasProperty undefinedProp true
ownProperty undefinedProp true

prop is deleted true
value undefined
hasProperty undefinedProp false
ownProperty undefinedProp false

prop is configured as 1 with value
value "1"
hasProperty undefinedProp true
ownProperty undefinedProp true

prop is deleted true
value undefined
hasProperty undefinedProp false
ownProperty undefinedProp false

prop is configured as 1 with getter|setter
value 1
hasProperty undefinedProp true
ownProperty undefinedProp true

prop is deleted true
value 1
hasProperty undefinedProp true
ownProperty undefinedProp true

prop is redeclared as 1 with assignment (=)
value "1"
hasProperty undefinedProp true
ownProperty undefinedProp true

prop is deleted true
value 1
hasProperty undefinedProp true
ownProperty undefinedProp true
@zero1five

This comment has been minimized.

Copy link
Contributor

commented May 31, 2019

This seems to me to be correct. by default, values added using Object.defineProperty() are immutable.

Delete Operator

edited: should not return true(delete env[prop]).

@Himself65

This comment has been minimized.

@addaleax

This comment has been minimized.

Copy link
Member

commented May 31, 2019

Fwiw, this is happening because the NamedPropertyHandlerConfiguration we use for process.env does not handle Object.defineProperty() (although we could add that), but we do handle delete and still actually attempt to delete the environment variable.

It’s not obvious to me what we should do when Object.defineProperty() with a getter/setter pair is used on process.env.

@addaleax addaleax added the process label May 31, 2019

@Himself65

This comment has been minimized.

Copy link
Contributor

commented May 31, 2019

where is code related code? I couldn't find...😕

@bnoordhuis

This comment has been minimized.

Copy link
Member

commented Jun 1, 2019

@Himself65 It's here:

node/src/node_env_var.cc

Lines 397 to 399 in aa8b820

env_proxy_template->SetHandler(NamedPropertyHandlerConfiguration(
EnvGetter, EnvSetter, EnvQuery, EnvDeleter, EnvEnumerator, data,
PropertyHandlerFlags::kHasNoSideEffect));

I agree with @addaleax that it's because of the missing GenericNamedPropertyDefinerCallback. There's an overload of NamedPropertyHandlerConfiguration that accepts a definer callback.

That said, accessors or non-configurable/enumerable/writable properties aren't meaningful on process.env. I think the logical course of action is to throw an exception or add a note to the docs saying "don't do that" - or both.

@Himself65

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2019

yes, I think no one will add a changeable value on process.env.🤔

but, I'm trying to add something to fix it

@Himself65 Himself65 referenced a pull request that will close this issue Jun 1, 2019

Open

process: disallow some uses of Object.defineProperty() on process.env #28006

3 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.