[JENKINS-70303] Trim leading/trailing whitespace from refspecs - #3989
Open
Racknaraock wants to merge 2 commits into
Open
[JENKINS-70303] Trim leading/trailing whitespace from refspecs#3989Racknaraock wants to merge 2 commits into
Racknaraock wants to merge 2 commits into
Conversation
A refspec with a leading space is not recognized as a force-update spec by JGit's RefSpec parser (the leading space hides the '+'), and the untrimmed spec can also be split into a spurious empty refspec by GitSCM's config-based fetch refspec handling. Trim the refspec at the two points where user-supplied refspec strings are first converted into RefSpec/config values.
Covers both fixed entry points: UserRemoteConfig's constructor (classic GitSCM / Freestyle) and GitSCMBuilder.asRefSpecs() (multibranch GitSCMSource).
Closed
9 tasks
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
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.
[JENKINS-70303] Trim leading/trailing whitespace from refspecs
Problem
When a user-supplied refspec has a leading space (e.g.
" +refs/heads/master:refs/remotes/origin/master"), JGit'sRefSpecparser fails to recognize the+force-update marker because the space hides it, and the checkout fails with "Remote does not have available for fetch." Command-line git tolerates this kind of extra whitespace; JGit does not.Reported in JENKINS-70303 against Jenkins 2.375.1 / git plugin 4.14.3 / git client plugin 3.13.1 and jenkinsci/git-client-plugin#1663
Related prior work
Both prior attempts targeted git-client-plugin (the lower-level git client abstraction), not git-plugin:
JGitAPIImpl.javaonly. Closed for inactivity after review feedback asking for: a link to the issue, an automated test, and fixing a new spotbugs warning it introduced. None of that was addressed before it went stale.GitAPI.java,CliGitAPIImpl.java,JGitAPIImpl.java,LegacyCompatibleGitAPIImpl.javaandRemoteGitImpl.java, with tests, and with maintainer (Mark Waite) commits directly on the branch. Still open as of this writing, opened January 2024, no activity since March 2024 — stalled despite being close to mergeable.This PR takes a different, narrower approach: instead of patching the git-client abstraction (CLI and/or JGit implementations), it trims the refspec at the two points in git-plugin where a user-supplied refspec string is first turned into a
RefSpec/config value, before it ever reaches git-client:UserRemoteConfigconstructor (classic FreestyleGitSCM/ data-bound form submission)GitSCMBuilder.asRefSpecs()(multibranchGitSCMSource)This covers both entry points with a much smaller diff, doesn't touch the JGit/CLI client layer at all (so it doesn't depend on #1096 landing), and avoids the spotbugs issue that sank #1095.
Changes
UserRemoteConfig.java: usefixEmptyAndTrim(refspec)instead offixEmpty(refspec)in the constructor.GitSCMBuilder.java: callrefSpec.trim()before constructing eachRefSpecinasRefSpecs().Testing
New unit tests covering both fixed entry points:
UserRemoteConfigRefSpecTest.constructorTrimsLeadingAndTrailingWhitespaceFromRefspecGitSCMBuilderTest.withRefSpecLeadingAndTrailingWhitespaceTargeted test run (
UserRemoteConfigRefSpecTest,GitSCMBuilderTest): green.Full
mvn compile test-compile: clean, no errors.Coverage (
mvn -P enable-jacoco test jacoco:report): both modified lines are fully exercised (0 missed instructions) —UserRemoteConfig.java:60andGitSCMBuilder.java:442. File-level:GitSCMBuilder.java96.4% instruction / 96.5% line coverage;UserRemoteConfig.java26.0% instruction / 27.1% line coverage (this class has many unrelated getters/validators not exercised by this slice of the suite).Manual verification on a local
mvn hpi:runJenkins instance: configured a Freestyle job against a local repo with a refspec containing leading/trailing spaces. Confirmed in the savedconfig.xmlthat the stored refspec is trimmed (+refs/heads/feature-branch:refs/remotes/origin/feature-branch, no surrounding whitespace), and the checkout completed successfully.Checklist (per CONTRIBUTING.adoc)