Fold groovy-sandbox into script-security-plugin - #926
Draft
gbhat618 wants to merge 271 commits into
Draft
Conversation
Looks like Groovy is failing to handle var arg correctly if the actual arguments that match the vararg is a single null value. Instead of creating [null], it just passes in null.
Groovy being dynamic language means it cannot properly handle the situation when a single Object[] argument is given where a function expects var arg
int foo(Object... args) { return args.length }
In Java,
foo(new Object[3])==3
foo((Object)new Object[3])==1
but Groovy naturally cannot do this. The use of var-args in Checker.checkedCall method was triggering this issue.
I removed var args from these methods to avoid the problem. Unfortunately Groovy doesn't support literal array syntax of
new Object[]{1,2,3}, so this involves creating a list only to convert to an array.
I'm resolving this issue by not passing such calls to interceptors. While internally I discovered that Groovy handles null.foo() as if NullObject.INSTANCE.foo(), but I don't think there's value in making interceptors aware of this weird case. as null LHS is unlikely to require any protection, this should be safe.
Migrate all Groovy test sources to Java
…closures Fix interception of return statements in closures
Use plugin parent POM
…s that use compound operators
Co-authored-by: Carroll Chiou <cchiou@cloudbees.com>
Bumps [git-changelist-maven-extension](https://github.com/jenkinsci/incrementals-tools) from 1.4 to 1.6. - [Release notes](https://github.com/jenkinsci/incrementals-tools/releases) - [Commits](jenkinsci/incrementals-tools@parent-1.4...parent-1.6) --- updated-dependencies: - dependency-name: io.jenkins.tools.incrementals:git-changelist-maven-extension dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
…jenkinsci#60) * Declare closure parameters when visiting closures Fixes jenkinsci#59 * Avoid calling super on types where that results in expressions being visited twice * Revert unnecessary changes to make the PR more focused
…TrackingClassCodeExpressionTransformer
[JENKINS-70080] Do not generate invalid bytecode for field assignments that use compound operators
…enkins.tools.incrementals-git-changelist-maven-extension-1.6 Bump git-changelist-maven-extension from 1.4 to 1.6
Co-authored-by: Devin Nusbaum <dwnusbaum@users.noreply.github.com>
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.
This PR merges the https://github.com/jenkinsci/groovy-sandbox into
script-security-plugin(here) as this is the single consumer(*) and the sole way of shipping any changes in groovy-sandbox. Keeping thegroovy-sandboxadded friction in delivering bug fixes. Especially security fixes needed very careful attention to stage thegroovy-sandboxrelease, and then consume it inscript-security-pluginand stagescript-security-pluginrelease.This PR folds the
groovy-sandboxlibrary into a folder here with the same namegroovy-sandbox/. The existingscript-security-pluginsource code is moved toplugin/(*) Though there is
workflow-cps/lib/which is one other (optional) user forgroovy-sandboxthis change still publishesgroovy-sandboxso it should be no changes to consumer.https://github.com/jenkinsci/workflow-cps-plugin/blob/30c8c00684a37764a1083dd40b496ccdccc90dd7/lib/pom.xml#L58-L63
Procedure
The following 3 commits are the important, below are details of how they are produced.
script-security-pluginsource code intoplugin/folder, introduced the parentpom.xmland adjusted the relationship betweenplugin/pom.xml.mvn clean packageworks.groovy-sandbox/doesn't fix the module yet.groovy-sandbox/and adjusts all 3 pom.xml to make it locally built, and consumed.Checklist
jenkins-infra/repository-permissions-updaterPRjenkinsci/groovy-sandboxremove source code and update READMEjenkinsci/groovy-sandboxon GitHubTesting done
mvn clean verifypasses, all tests of bothgroovy-sandboxandpluginare executed.Submitter checklist