Fix GRAV_CONFIG env override gate to also check $_SERVER/$_ENV - #4285
Closed
wakqasahmed wants to merge 1 commit into
Closed
Fix GRAV_CONFIG env override gate to also check $_SERVER/$_ENV#4285wakqasahmed wants to merge 1 commit into
wakqasahmed wants to merge 1 commit into
Conversation
…etenv() Some SAPIs (Apache SetEnv, nginx fastcgi_param) only populate $_SERVER, never the process environment getenv() reads. The gate at InitializeProcessor::initializeConfig() checked getenv() alone even though the body one line below already reads $_ENV + $_SERVER, so the whole GRAV_CONFIG__* override feature silently did nothing under those setups. Setup.php had the same getenv()-only pattern for GRAV_ENVIRONMENT, GRAV_SETUP_PATH, GRAV_ENVIRONMENT_PATH and GRAV_ENVIRONMENTS_PATH. Applies the same $_SERVER ?? $_ENV ?? getenv() fallback Env.php already uses (and already has test coverage for) at all five call sites. Added a regression test that fails on the old code and passes on the fix. Fixes getgrav#4279
Contributor
Author
|
Closing in favor of #4286 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
GRAV_CONFIGoverride gate inInitializeProcessor::initializeConfig()checksgetenv($prefix), but the body one line below already reads$_ENV + $_SERVER. On Apache withSetEnvor nginx withfastcgi_param, the variable lands in$_SERVERbut never in the process environmentgetenv()reads, so the gate is false and the wholeGRAV_CONFIG__*override feature does nothing — same failure mode as #4260/#4275.Setup.phphas the identical pattern forGRAV_ENVIRONMENT,GRAV_SETUP_PATH,GRAV_ENVIRONMENT_PATHandGRAV_ENVIRONMENTS_PATH.Env.phpalready solves this correctly ($_SERVER[$key] ?? $_ENV[$key] ?? (getenv($key) ?: null)) and has its own test coverage proving the precedence. This PR applies the same fallback at all five call sites instead of inventing a new pattern.Added a test that sets the override only in
$_SERVERand drives the realinitializeConfig()— confirmed it fails against the old code and passes against the fix. Ran the fullInitializeProcessorTestandEnvTestsuites after, both green (59 tests).Fixes #4279