[Fix JENKINS-23411] Throws Failure if the item does not exist (#3007)

* [Fix JENKINS-23411] Throws Failure if the item does not exist * [Fix JENKINS-23411] Adress wording in error message
jenkinsci · Sep 22, 2017 · fddffb4 · fddffb4
1 parent 27569a9
commit fddffb4
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 1 deletion.
diff --git a/core/src/main/java/hudson/model/ListView.java b/core/src/main/java/hudson/model/ListView.java
@@ -45,6 +45,7 @@
 import java.util.regex.Pattern;
 import java.util.regex.PatternSyntaxException;
 
+import javax.annotation.CheckForNull;
 import javax.annotation.concurrent.GuardedBy;
 import javax.servlet.ServletException;
 import jenkins.model.Jenkins;
@@ -378,13 +379,16 @@ public HttpResponse doRemoveJobFromView(@QueryParameter String name) throws IOEx
             throw new Failure("Query parameter 'name' is required");
 
         TopLevelItem item = resolveName(name);
+        if (item==null)
+            throw new Failure("Query parameter 'name' does not correspond to a known and readable item");
+
         if (remove(item))
             owner.save();
 
         return HttpResponses.ok();
     }
 
-    private TopLevelItem resolveName(String name) {
+    private @CheckForNull TopLevelItem resolveName(String name) {
         TopLevelItem item = getOwner().getItemGroup().getItem(name);
         if (item == null) {
             name = Items.getCanonicalName(getOwner().getItemGroup(), name);

diff --git a/test/src/test/java/hudson/model/ListViewTest.java b/test/src/test/java/hudson/model/ListViewTest.java
@@ -259,6 +259,32 @@ private void checkLinkFromItemExistsAndIsValid(Item item, ItemGroup ig, Item top
         assertEquals("job1", items.get(0).getName());
     }
 
+    @Issue("JENKINS-23411")
+    @Test public void doRemoveJobFromViewNullItem() throws Exception {
+        MockFolder folder = j.createFolder("folder");
+        ListView view = new ListView("view", folder);
+        folder.addView(view);
+        FreeStyleProject job = folder.createProject(FreeStyleProject.class, "job1");
+        view.add(job);
+
+        List<TopLevelItem> items = view.getItems();
+        assertEquals(1, items.size());
+        assertEquals("job1", items.get(0).getName());
+
+        // remove a contained job
+        view.doRemoveJobFromView("job1");
+        List<TopLevelItem> itemsNow = view.getItems();
+        assertEquals(0, itemsNow.size());
+
+        // remove a not contained job
+        try {
+            view.doRemoveJobFromView("job2");
+            fail("Remove job2");
+        } catch(Failure e) {
+            assertEquals(e.getMessage(), "Query parameter 'name' does not correspond to a known item");
+        }
+    }
+
     private static class AllButViewsAuthorizationStrategy extends AuthorizationStrategy {
         @Override public ACL getRootACL() {
             return UNSECURED.getRootACL();