Skip to content

Fix recursive env var expansion in EnvVars.resolve() (JENKINS-33239)#26248

Open
meetgoti07 wants to merge 5 commits intojenkinsci:masterfrom
meetgoti07:fix/replacemacro_recursion
Open

Fix recursive env var expansion in EnvVars.resolve() (JENKINS-33239)#26248
meetgoti07 wants to merge 5 commits intojenkinsci:masterfrom
meetgoti07:fix/replacemacro_recursion

Conversation

@meetgoti07
Copy link

@meetgoti07 meetgoti07 commented Feb 3, 2026

Fix recursive env var expansion and flaky FormValidationTest

Exclude the variable being expanded from the resolver so self-references are left as literal and no longer expand recursively.
Also fixed FormValidationTest.testUrlCheck to mock the network call to jenkins.io, preventing flaky failures when the external site is unavailable (503 errors).

Fixes: #21801

Testing done

Reverted the fix and ran EnvVarsTest#resolveSelfReferenceDoesNotGrowUnbounded to confirm failure (wrong resolved value). Restored the fix and re-ran the test to confirm pass. Added resolveSelfReferenceDoesNotGrowUnbounded in EnvVarsTest and replaceMacroSelfReferenceDoesNotGrowUnbounded in UtilTest; both are executed by the PR build.
Verified FormValidationTest.testUrlCheck passes consistently with the new mock implementation.

Screenshots (UI changes only)

N/A (core env resolution only).

Before (test output without fix)

[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running hudson.EnvVarsTest
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.039 s <<< FAILURE! -- in hudson.EnvVarsTest
[ERROR] hudson.EnvVarsTest.resolveSelfReferenceDoesNotGrowUnbounded -- Time elapsed: 0.030 s <<< FAILURE!
org.opentest4j.AssertionFailedError: ORACLE_HOME is expanded; self-reference $LD_LIBRARY_PATH is left as literal ==> expected: </opt/oracle/lib:$LD_LIBRARY_PATH> but was: </opt/oracle/lib:$ORACLE_HOME/lib:$LD_LIBRARY_PATH>
        at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:158)
        at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:139)
        at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:201)
        at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:184)
        at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1199)
        at hudson.EnvVarsTest.resolveSelfReferenceDoesNotGrowUnbounded(EnvVarsTest.java:185)

[INFO] 
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   EnvVarsTest.resolveSelfReferenceDoesNotGrowUnbounded:185 ORACLE_HOME is expanded; self-reference $LD_LIBRARY_PATH is left as literal ==> expected: </opt/oracle/lib:$LD_LIBRARY_PATH> but was: </opt/oracle/lib:$ORACLE_HOME/lib:$LD_LIBRARY_PATH>
[INFO] 
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  4.666 s
[INFO] Finished at: 2026-02-03T11:06:46+05:30
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.5.4:test (default-test) on project jenkins-core: There are test failures.
[ERROR] 
[ERROR] See /Users/meetgoti/GSOC26/jenkins/core/target/surefire-reports for the individual test results.
[ERROR] See dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream.
[ERROR] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to get full stack traces.
[ERROR] 
[ERROR] For more information, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException

After (test output with fix)

[INFO] Running hudson.EnvVarsTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.030 s -- in hudson.EnvVarsTest
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  3.609 s
[INFO] Finished at: 2026-02-03T11:07:37+05:30
[INFO] ------------------------------------------------------------------------

Proposed changelog entries

  • Prevent recursive expansion of self-referencing environment variables in EnvVars.resolve() (e.g. PATH=path1:$PATH, LD_LIBRARY_PATH=$ORACLE_HOME/lib:$LD_LIBRARY_PATH) so values no longer grow unbounded and cause OOM.

Proposed changelog category

/label bug

Proposed upgrade guidelines

N/A

Submitter checklist

  • The issue, if it exists, is well-described.
  • The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples). Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
  • There is automated testing or an explanation as to why this change has no tests.
  • New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
  • New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
  • UI changes do not introduce regressions when enforcing the current default rules of Content Security Policy Plugin. In particular, new or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
  • For dependency updates, there are links to external changelogs and, if possible, full differentials.
  • For new APIs and extension points, there is a link to at least one consumer.

Desired reviewers

@MarkEWaite

Before the changes are marked as ready-for-merge:

Maintainer checklist

  • There are at least two (2) approvals for the pull request and no outstanding requests for change.
  • Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
  • Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
  • Proper changelog labels are set so that the changelog can be generated automatically.
  • If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
  • If it would make sense to backport the change to LTS, be a Bug or Improvement, and either the issue or pull request must be labeled as lts-candidate to be considered.

When resolving env vars, the value of a variable was expanded using the
same map as the resolver. Self-references (e.g. PATH=path1:/Users/meetgoti/GSOC26/.direnv/python-3.10/bin:/Users/meetgoti/.pyenv/plugins/pyenv-virtualenv/shims:/Users/meetgoti/.pyenv/shims:/Users/meetgoti/.pyenv/bin:/opt/apache-maven/bin:/Library/Java/JavaVirtualMachines/temurin-21.jdk/Contents/Home/bin:/Users/meetgoti/.antigravity/antigravity/bin:/opt/homebrew/opt/bison/bin:/opt/homebrew/opt/flex/bin:/opt/homebrew/opt/openjdk/bin:/Users/meetgoti/.nvm/versions/node/v24.11.1/bin:/Users/meetgoti/.cursor/extensions/vadimcn.vscode-lldb-1.12.0/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/opt/pmk/env/global/bin:/Library/Apple/usr/bin:/Library/TeX/texbin:/Applications/Postgres.app/Contents/Versions/latest/bin:/Users/meetgoti/.cursor/extensions/vadimcn.vscode-lldb-1.12.0/bin:/Users/meetgoti/.pyenv/plugins/pyenv-virtualenv/shims:/Users/meetgoti/.pyenv/bin:/opt/apache-maven/bin:/Library/Java/JavaVirtualMachines/temurin-21.jdk/Contents/Home/bin:/Users/meetgoti/.antigravity/antigravity/bin:/opt/homebrew/opt/bison/bin:/opt/homebrew/opt/flex/bin:/Users/meetgoti/.local/bin:/opt/homebrew/opt/openjdk/bin:/opt/homebrew/Caskroom/miniconda/base/bin:/opt/homebrew/Caskroom/miniconda/base/condabin:/Users/meetgoti/Library/pnpm:/Users/meetgoti/.nvm/versions/node/v24.11.1/bin:/Users/meetgoti/.cargo/bin:/opt/homebrew/share/google-cloud-sdk/bin:/Users/meetgoti/go/bin:/opt/homebrew/share/google-cloud-sdk/bin:/Users/meetgoti/go/bin or
LD_LIBRARY_PATH=/lib:) could then be expanded
repeatedly and cause unbounded growth / OOM.

Exclude the variable being expanded from the resolver so self-references
are left as literal and no longer expand recursively.
@comment-ops-bot comment-ops-bot bot added the bug For changelog: Minor bug. Will be listed after features label Feb 3, 2026
@meetgoti07 meetgoti07 marked this pull request as ready for review February 3, 2026 05:52
@meetgoti07 meetgoti07 marked this pull request as draft February 3, 2026 08:49
The [testUrlCheck](cci:1://file:///Users/meetgoti/GSOC26/jenkins/core/src/test/java/hudson/util/FormValidationTest.java:123:4-142:5) test was failing intermittently because it made a real network request to `https://www.jenkins.io/`, causing build failures when the external site was unavailable (e.g., 503 Service Unavailable).

This change refactors the test to use a mock response for the `open()` method, ensuring the test validates the logic independently of external network conditions.
@meetgoti07 meetgoti07 marked this pull request as ready for review February 3, 2026 17:01
@meetgoti07
Copy link
Author

@MarkEWaite, when you have time, could you please review this PR?
Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug For changelog: Minor bug. Will be listed after features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[JENKINS-33239] Environment variable resolves variables recursively

1 participant