From df2b7d97ad8018d8fc4929b8a24bc8bcbfb90926 Mon Sep 17 00:00:00 2001
From: Christopher Simons <christopherleesimons@gmail.com>
Date: Tue, 8 Dec 2015 00:33:47 -0500
Subject: [PATCH] [FIXED JENKINS-31321] protect against node-rename corruption

This change adds code to check that the user isn't attempting to rename
an existing node with the name of another existing node.  Previous to
this change, such rename operations would succeed and would corrupt node
configuration data.

(cherry picked from commit 16d6429c58400b18350c42280ac541a6322ac8f3)
---
 core/src/main/java/hudson/model/Computer.java | 13 +++++--
 .../test/java/hudson/model/ComputerTest.java  | 37 ++++++++++++++++++-
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java
index e30f5c390a..4ce4a3b7a3 100644
--- a/core/src/main/java/hudson/model/Computer.java
+++ b/core/src/main/java/hudson/model/Computer.java
@@ -2,7 +2,8 @@
  * The MIT License
  *
  * Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi,
- * Red Hat, Inc., Seiji Sogabe, Stephen Connolly, Thomas J. Black, Tom Huybrechts, CloudBees, Inc.
+ * Red Hat, Inc., Seiji Sogabe, Stephen Connolly, Thomas J. Black, Tom Huybrechts,
+ * CloudBees, Inc., Christopher Simons
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -1384,13 +1385,19 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces
     public void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException, FormException {
         checkPermission(CONFIGURE);
 
-        String name = Util.fixEmptyAndTrim(req.getSubmittedForm().getString("name"));
-        Jenkins.checkGoodName(name);
+        String proposedName = Util.fixEmptyAndTrim(req.getSubmittedForm().getString("name"));
+        Jenkins.checkGoodName(proposedName);
 
         Node node = getNode();
         if (node == null) {
             throw new ServletException("No such node " + nodeName);
         }
+
+        if ((!proposedName.equals(nodeName))
+                && Jenkins.getActiveInstance().getNode(proposedName) != null) {
+            throw new FormException(Messages.ComputerSet_SlaveAlreadyExists(proposedName), "name");
+        }
+
         Node result = node.reconfigure(req, req.getSubmittedForm());
         replaceBy(result);
 
diff --git a/test/src/test/java/hudson/model/ComputerTest.java b/test/src/test/java/hudson/model/ComputerTest.java
index b58b00f45e..8b6efad413 100644
--- a/test/src/test/java/hudson/model/ComputerTest.java
+++ b/test/src/test/java/hudson/model/ComputerTest.java
@@ -1,7 +1,7 @@
 /*
  * The MIT License
  *
- * Copyright (c) 2015 Red Hat, Inc.
+ * Copyright (c) 2015 Red Hat, Inc.; Christopher Simons
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -23,16 +23,26 @@
  */
 package hudson.model;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.*;
 
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
+import com.gargoylesoftware.htmlunit.html.HtmlForm;
+import com.gargoylesoftware.htmlunit.html.HtmlPage;
+
 import java.io.File;
 
 import jenkins.model.Jenkins;
 import hudson.slaves.DumbSlave;
 
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.JenkinsRule.WebClient;
 
 public class ComputerTest {
 
@@ -52,4 +62,29 @@ public class ComputerTest {
 
         assertTrue("Slave log should be kept", keep.toComputer().getLogFile().exists());
     }
+
+    /**
+     * Verify we can't rename a node over an existing node.
+     */
+    @Issue("JENKINS-31321")
+    @Test
+    public void testProhibitRenameOverExistingNode() throws Exception {
+        final String NOTE = "Rename node to name of another node should fail.";
+
+        Node nodeA = j.createSlave("nodeA", null, null);
+        Node nodeB = j.createSlave("nodeB", null, null);
+
+        WebClient wc = j.createWebClient();
+        HtmlForm form = wc.getPage(nodeB, "configure").getFormByName("config");
+        form.getInputByName("_.name").setValueAttribute("nodeA");
+
+        try {
+            j.submit(form);
+            fail(NOTE);
+        } catch (FailingHttpStatusCodeException e) {
+            assertThat(NOTE, e.getStatusCode(), equalTo(400));
+            assertThat(NOTE, e.getResponse().getContentAsString(),
+                    containsString("Slave called ‘nodeA’ already exists"));
+        }
+    }
 }
-- 
GitLab