Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-63355] Prevent NPEs when library name is not provided #101

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,17 @@

@DataBoundConstructor public LibraryConfiguration(String name, LibraryRetriever retriever) {
this.name = Util.fixEmptyAndTrim(name);
if(this.name == null) {
throw new IllegalArgumentException("You must enter a library name");
}
this.retriever = retriever;
}

/**
* Library name.
* Should match {@link Library#value}, up to the first occurrence of {@code @}, if any.
*/
@NonNull
public String getName() {
return name;
}
Expand Down Expand Up @@ -173,6 +177,9 @@

@RequirePOST
public FormValidation doCheckDefaultVersion(@AncestorInPath Item context, @QueryParameter String defaultVersion, @QueryParameter boolean implicit, @QueryParameter boolean allowVersionOverride, @QueryParameter String name) {
if (name.isEmpty()) {

Check warning on line 180 in src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryConfiguration.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 180 is only partially covered, one branch is missing
return FormValidation.error("You must enter a name.");

Check warning on line 181 in src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryConfiguration.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 181 is not covered by tests
}
if (defaultVersion.isEmpty()) {
if (implicit) {
return FormValidation.error("If you load a library implicitly, you must specify a default version.");
Expand All @@ -184,7 +191,7 @@
} else {
for (LibraryResolver resolver : ExtensionList.lookup(LibraryResolver.class)) {
for (LibraryConfiguration config : resolver.fromConfiguration(Stapler.getCurrentRequest())) {
if (config.getName().equals(name)) {
if (name.equals(config.getName())) {

Check warning on line 194 in src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryConfiguration.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 194 is only partially covered, one branch is missing
Copy link
Member

Choose a reason for hiding this comment

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

could be reverted

return config.getRetriever().validateVersion(name, defaultVersion, context);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,9 @@
TaskListener listener = getContext().get(TaskListener.class);
String[] parsed = LibraryAdder.parse(step.identifier);
String name = parsed[0], version = parsed[1];
if (name == null) {

Check warning on line 180 in src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 180 is only partially covered, one branch is missing
throw new AbortException("No library name provided");

Check warning on line 181 in src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 181 is not covered by tests
}
boolean trusted = false;
Boolean changelog = step.getChangelog();
LibraryCachingConfiguration cachingConfiguration = null;
Expand All @@ -186,7 +189,7 @@
if (retriever == null) {
for (LibraryResolver resolver : ExtensionList.lookup(LibraryResolver.class)) {
for (LibraryConfiguration cfg : resolver.forJob(run.getParent(), Collections.singletonMap(name, version))) {
if (cfg.getName().equals(name)) {
if (name.equals(cfg.getName())) {
Copy link
Member

Choose a reason for hiding this comment

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

ditto

retriever = cfg.getRetriever();
trusted = resolver.isTrusted();
version = cfg.defaultedVersion(version);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ THE SOFTWARE.
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form">
<f:entry field="identifier" title="${%Name and optional version}">
<f:textbox/>
<f:textbox clazz="required"/>
</f:entry>
<f:entry field="changelog" title="${%Include recent changes in jobs}">
<f:checkbox default="true"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

package org.jenkinsci.plugins.workflow.libs;

import java.util.Collections;
import java.util.List;

import hudson.plugins.git.GitSCM;
import org.hamcrest.Matchers;
Expand Down Expand Up @@ -69,29 +69,24 @@ public class LibraryConfigurationTest {
}

@Issue("JENKINS-59527")
@Test public void emptyStringDefaultVersionAndName() {
String libraryName = "";
String defaultVersion = "";

LibraryConfiguration cfg = new LibraryConfiguration(libraryName, new SCMRetriever(new GitSCM("https://phony.jenkins.io/bar.git")));
cfg.setDefaultVersion(defaultVersion);

assertNull(cfg.getName());
@Test public void emptyDefaultVersion() {
LibraryConfiguration cfg = new LibraryConfiguration("test", new SCMRetriever(new GitSCM("https://phony.jenkins.io/bar.git")));
cfg.setDefaultVersion("");
assertNull(cfg.getDefaultVersion());
}

@Issue("JENKINS-59527")
@Test public void nullDefaultVersionAndName() {
String libraryName = null;
String defaultVersion = null;

LibraryConfiguration cfg = new LibraryConfiguration(libraryName, new SCMRetriever(new GitSCM("https://phony.jenkins.io/bar.git")));
cfg.setDefaultVersion(defaultVersion);

assertNull(cfg.getName());
cfg = new LibraryConfiguration("test", new SCMRetriever(new GitSCM("https://phony.jenkins.io/bar.git")));
cfg.setDefaultVersion(null);
assertNull(cfg.getDefaultVersion());
}


@Issue("JENKINS-63355")
@Test public void emptyNameCannotBeSaved() throws Exception {
assertThrows(IllegalArgumentException.class, () -> GlobalLibraries.get().setLibraries(List.of(
new LibraryConfiguration(null, new SCMRetriever(new GitSCM("https://phony.jenkins.io/bar.git")))
)));
assertThrows(IllegalArgumentException.class, () -> GlobalLibraries.get().setLibraries(List.of(
new LibraryConfiguration("", new SCMRetriever(new GitSCM("https://phony.jenkins.io/bar.git")))
)));
}

}