[core] Disable local level of gEnv#22775
Conversation
The previous fix for this made incorrect use of the PrependPathName() API. Fixing.
jblomer
force-pushed
the
restrict-rclogon-v2
branch
from
6950a4d to
b8076f5
Compare
Add a new option for constructing TEnv that allows to disable the local level. If disabled, configuration cannot be read anymore into the local level, the local level cannot be saved and values cannot be set in the local level. Effectively, the local level is always empty in this mode. By default, new instances of TEnv have the local level _enabled_. Only gEnv has it disabled to prevent environment poisoning. The old behavior can be restored by setting the ROOTENV_UNSAFE_USE_LOCAL environment variable to a non-empty string.
Test Results 23 files 23 suites 3d 14h 24m 21s ⏱️ For more details on these failures, see this check. Results for commit b8076f5. |
| global file is `$ROOTSYS/etc/system<name>` (or `ROOTETCDIR/system<name>`) | ||
| the user file is `$HOME/<name>` and the local file is `./<name>`. | ||
|
|
||
| For gEnv, the local level is disabled to prevent environment poisoning. |
There was a problem hiding this comment.
I think it's a bit obscure what we mean by "environment poisoning". Could we make this comment a little more explicit on why it is a safety measure?
| @@ -24,7 +24,7 @@ class TWriteEnvParser; | |||
| enum EEnvLevel { | |||
There was a problem hiding this comment.
It would be nice to add a brief explanation of what each level means
| Bool_t fIgnoreDup; // ignore duplicates, don't issue warning | ||
| THashList *fTable; // hash table containing env records | ||
| TString fRcName; // resource file base name | ||
| Bool_t fIgnoreDup; // ignore duplicates, don't issue warning |
There was a problem hiding this comment.
We should probably initialize all these fields inline
|
|
||
| For gEnv, the local level is disabled to prevent environment poisoning. | ||
| The behavior can be changed to be backwards-compatible by setting | ||
| `ROOTENV_UNSAFE_USE_LOCAL=1`. |
There was a problem hiding this comment.
Intuitively I would think that ROOTENV_UNSAFE_USE_LOCAL=0 would mean "explicitly disable local env". It would be beneficial to either make it that way or warn the user if we find a value 0 (or false) inside this variable that it's not doing what they think.
2 participants
Add a new option for constructing TEnv that allows to disable the local
level. If disabled, configuration cannot be read anymore into the local
level, the local level cannot be saved and values cannot be set in the
local level. Effectively, the local level is always empty in this mode.
By default, new instances of TEnv have the local level enabled. Only
gEnv has it disabled to prevent environment poisoning.
The old behavior can be restored by setting the
ROOTENV_UNSAFE_USE_LOCALenvironment variable to a non-empty string.