Skip to content

Commit

Permalink
[FIXED JENKINS-19852] NPE during submission of EnvInject JobProperty …
Browse files Browse the repository at this point in the history 
…configurations
  • Loading branch information
@christ66
christ66 committed May 19, 2014
1 parent c68558a commit d6fbc76
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/main/java/org/jenkinsci/plugins/envinject/EnvInjectJobProperty.java
View file
Expand Up @@ -126,7 +126,7 @@ public JobProperty<?> reconfigure(StaplerRequest req, JSONObject form) throws De
// Don't let non RUN_SCRIPT users set arbitrary groovy script
property.info = new EnvInjectJobPropertyInfo(property.info.propertiesFilePath, property.info.propertiesContent,
property.info.getScriptFilePath(), property.info.getScriptContent(),
this.info.getGroovyScriptContent(),
property.info.getGroovyScriptContent(),
property.info.isLoadFilesFromMaster());
}
return property;
Expand Down

2 comments on commit d6fbc76

@recampbell
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@christ66, I realizes this "fixes" the NPE, but I wonder if it breaks what @ndeloof was trying to accomplish in 96a5269 in the first place -- to prevent users without RUN_SCRIPTS from editing the groovy script.

Note that the groovy script is read-only without the RUN_SCRIPTS permission, but one could trivially edit the HTML to inject whatever groovy they like. So the server side check is still needed.

My point is, I think we need to re-open JENKINS-19852 and let @ndeloof fix the NPE in a way which still prevents editing of the groovy in this case as desired in 96a5269.

@ndeloof
Copy link
Contributor

@ndeloof ndeloof commented on d6fbc76 Jun 2, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed by e920857

Please sign in to comment.