From a92281637f5b629e110b5bd074566bb6c302bb62 Mon Sep 17 00:00:00 2001
From: Jessica Frazelle <jess@docker.com>
Date: Tue, 13 Jan 2015 17:30:49 -0800
Subject: [PATCH] Renaming a container with an invalid name should fail

Docker-DCO-1.1-Signed-off-by: Jessica Frazelle <jess@docker.com> (github: jfrazelle)
---
 daemon/rename.go                          | 22 +++++++++++++---------
 integration-cli/docker_cli_rename_test.go | 20 ++++++++++++++++++++
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/daemon/rename.go b/daemon/rename.go
index 9b030aad0e..1dedc7d3a7 100644
--- a/daemon/rename.go
+++ b/daemon/rename.go
@@ -8,23 +8,27 @@ func (daemon *Daemon) ContainerRename(job *engine.Job) engine.Status {
 	if len(job.Args) != 2 {
 		return job.Errorf("usage: %s OLD_NAME NEW_NAME", job.Name)
 	}
-	old_name := job.Args[0]
-	new_name := job.Args[1]
+	oldName := job.Args[0]
+	newName := job.Args[1]
 
-	container := daemon.Get(old_name)
+	container := daemon.Get(oldName)
 	if container == nil {
-		return job.Errorf("No such container: %s", old_name)
+		return job.Errorf("No such container: %s", oldName)
 	}
 
+	oldName = container.Name
+
 	container.Lock()
 	defer container.Unlock()
-	if err := daemon.containerGraph.Delete(container.Name); err != nil {
-		return job.Errorf("Failed to delete container %q: %v", old_name, err)
-	}
-	if _, err := daemon.reserveName(container.ID, new_name); err != nil {
+	if _, err := daemon.reserveName(container.ID, newName); err != nil {
 		return job.Errorf("Error when allocating new name: %s", err)
 	}
-	container.Name = new_name
+
+	container.Name = newName
+
+	if err := daemon.containerGraph.Delete(oldName); err != nil {
+		return job.Errorf("Failed to delete container %q: %v", oldName, err)
+	}
 
 	return engine.StatusOK
 }
diff --git a/integration-cli/docker_cli_rename_test.go b/integration-cli/docker_cli_rename_test.go
index 3ba98e4e37..911bab25ab 100644
--- a/integration-cli/docker_cli_rename_test.go
+++ b/integration-cli/docker_cli_rename_test.go
@@ -97,3 +97,23 @@ func TestRenameCheckNames(t *testing.T) {
 
 	logDone("rename - running container")
 }
+
+func TestRenameInvalidName(t *testing.T) {
+	defer deleteAllContainers()
+	runCmd := exec.Command(dockerBinary, "run", "--name", "myname", "-d", "busybox", "top")
+	if out, _, err := runCommandWithOutput(runCmd); err != nil {
+		t.Fatalf(out, err)
+	}
+
+	runCmd = exec.Command(dockerBinary, "rename", "myname", "new:invalid")
+	if out, _, err := runCommandWithOutput(runCmd); err == nil || !strings.Contains(out, "Invalid container name") {
+		t.Fatalf("Renaming container to invalid name should have failed: %s\n%v", out, err)
+	}
+
+	runCmd = exec.Command(dockerBinary, "ps", "-a")
+	if out, _, err := runCommandWithOutput(runCmd); err != nil || !strings.Contains(out, "myname") {
+		t.Fatalf("Output of docker ps should have included 'myname': %s\n%v", out, err)
+	}
+
+	logDone("rename - invalid container name")
+}