Merge pull request #45 from jglick/NPE-JENKINS-57218

[JENKINS-57218] Better null safety
jenkinsci · Apr 30, 2019 · e2eeab5 · e2eeab5
2 parents f32e384 + 6b41022
commit e2eeab5
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 1 deletion.
diff --git a/plugin/src/main/java/org/jenkinsci/plugins/structs/SymbolLookup.java b/plugin/src/main/java/org/jenkinsci/plugins/structs/SymbolLookup.java
@@ -139,11 +139,15 @@ public Descriptor<?> findDescriptor(Class<?> type, String symbol) {
 
             for (Class<?> e : Index.list(Symbol.class, pluginManager.uberClassLoader, Class.class)) {
                 if (Descriptor.class.isAssignableFrom(e)) {
+                    Descriptor<?> d = jenkins.getDescriptorByType(e.asSubclass(Descriptor.class));
+                    if (d == null) {
+                        LOGGER.fine(() -> e.getName() + " is not registered as an extension, so will be ignored");
+                        continue;
+                    }
                     Symbol s = e.getAnnotation(Symbol.class);
                     if (s != null) {
                         for (String t : s.value()) {
                             if (t.equals(symbol)) {
-                                Descriptor d = (Descriptor) jenkins.getInjector().getInstance(e);
                                 if (type.isAssignableFrom(d.clazz)) {
                                     cache.put(k, d);
                                     return d;

diff --git a/plugin/src/test/java/org/jenkinsci/plugins/structs/SymbolLookupTest.java b/plugin/src/test/java/org/jenkinsci/plugins/structs/SymbolLookupTest.java
@@ -1,5 +1,6 @@
 package org.jenkinsci.plugins.structs;
 
+import hudson.model.AbstractDescribableImpl;
 import hudson.model.BooleanParameterValue;
 import hudson.model.Descriptor;
 import java.util.Collections;
@@ -12,6 +13,7 @@
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ErrorCollector;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.TestExtension;
@@ -26,6 +28,8 @@ public static class Bar {}
     @Rule
     public JenkinsRule rule = new JenkinsRule();
 
+    @Rule public ErrorCollector errors = new ErrorCollector();
+
     @Inject
     SymbolLookup lookup;
 
@@ -104,4 +108,32 @@ public void descriptorIsDescribable() {
     @Symbol("whatever")
     public static class SomeConfiguration extends GlobalConfiguration {}
 
+    @Issue("JENKINS-57218")
+    @Test public void descriptorSansExtension() throws Exception {
+        SymbolLookup sl = rule.jenkins.getExtensionList(SymbolLookup.class).get(0);
+        errors.checkThat("A is registered", sl.findDescriptor(Stuff.class, "a"), is(instanceOf(StuffA.DescriptorImpl.class)));
+        errors.checkThat("B is not", sl.findDescriptor(Stuff.class, "b"), nullValue());
+        errors.checkThat("C is, but the registration is broken", sl.findDescriptor(Stuff.class, "c"), nullValue());
+        errors.checkThat("A (cached)", sl.findDescriptor(Stuff.class, "a"), is(instanceOf(StuffA.DescriptorImpl.class)));
+        errors.checkThat("B (cached)", sl.findDescriptor(Stuff.class, "b"), nullValue());
+        errors.checkThat("C (cached)", sl.findDescriptor(Stuff.class, "c"), nullValue());
+    }
+    public static abstract class Stuff extends AbstractDescribableImpl<Stuff> {}
+    public static final class StuffA extends Stuff {
+        @Symbol("a")
+        @TestExtension("descriptorSansExtension") public static final class DescriptorImpl extends Descriptor<Stuff> {}
+    }
+    public static final class StuffB extends Stuff {
+        @Symbol("b")
+        /* no extension */ public static final class DescriptorImpl extends Descriptor<Stuff> {}
+    }
+    public static final class StuffC extends Stuff {
+        @Symbol("c")
+        @TestExtension("descriptorSansExtension") public static final class DescriptorImpl extends Descriptor<Stuff> {
+            public DescriptorImpl() {
+                throw new Error("oops");
+            }
+        }
+    }
+
 }