daemon: improve validation for container rename

Improve validation for empty name; while the daemon already handled empty
strings, it didn't account for the "canonical" name with "/" prefix, for
which it would produce an obscure error:

    Error response from daemon: Error when allocating new name: Invalid container name (/ ), only [a-zA-Z0-9][a-zA-Z0-9_.-] are allowed

Before this change:

    curl -XPOST --unix-socket /var/run/docker.sock 'http://localhost/v1.51/containers/old/rename?name='
    {"message":"Neither old nor new names may be empty"}

    curl -XPOST --unix-socket /var/run/docker.sock 'http://localhost/v1.51/containers/old/rename?name=/'
    {"message":"Error when allocating new name: Invalid container name (/), only [a-zA-Z0-9][a-zA-Z0-9_.-] are allowed"}

    curl -XPOST --unix-socket /var/run/docker.sock 'http://localhost/v1.51/containers/old/rename?name=/hello'
    # OK

A check was added in the client as well for situations where an older daemon
is used; the same code currently was implemented in the CLI.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

client/container_rename.go

@@ -3,6 +3,9 @@ package client
 import (
 	"context"
 	"net/url"
+	"strings"
+
+	"github.com/containerd/errdefs"
 )
 
 // ContainerRenameOptions represents the options for renaming a container.
@@ -21,6 +24,12 @@ func (cli *Client) ContainerRename(ctx context.Context, containerID string, opti
 	if err != nil {
 		return ContainerRenameResult{}, err
 	}
+	options.NewName = strings.TrimSpace(options.NewName)
+	if options.NewName == "" || strings.TrimPrefix(options.NewName, "/") == "" {
+		// daemons before v29.0 did not handle the canonical name ("/") well
+		// let's be nice and validate it here before sending
+		return ContainerRenameResult{}, errdefs.ErrInvalidArgument.WithMessage("new name cannot be blank")
+	}
 
 	query := url.Values{}
 	query.Set("name", options.NewName)

daemon/rename.go

@@ -18,7 +18,9 @@ import (
 // to find the container. An error is returned if newName is already
 // reserved.
 func (daemon *Daemon) ContainerRename(oldName, newName string) (retErr error) {
-	if oldName == "" || newName == "" {
+	oldName = strings.TrimSpace(oldName)
+	newName = strings.TrimSpace(newName)
+	if strings.TrimPrefix(oldName, "/") == "" || strings.TrimPrefix(newName, "/") == "" {
 		return errdefs.InvalidParameter(errors.New("Neither old nor new names may be empty"))
 	}
 

vendor/github.com/moby/moby/client/container_rename.go

@@ -3,6 +3,9 @@ package client
 import (
 	"context"
 	"net/url"
+	"strings"
+
+	"github.com/containerd/errdefs"
 )
 
 // ContainerRenameOptions represents the options for renaming a container.
@@ -21,6 +24,12 @@ func (cli *Client) ContainerRename(ctx context.Context, containerID string, opti
 	if err != nil {
 		return ContainerRenameResult{}, err
 	}
+	options.NewName = strings.TrimSpace(options.NewName)
+	if options.NewName == "" || strings.TrimPrefix(options.NewName, "/") == "" {
+		// daemons before v29.0 did not handle the canonical name ("/") well
+		// let's be nice and validate it here before sending
+		return ContainerRenameResult{}, errdefs.ErrInvalidArgument.WithMessage("new name cannot be blank")
+	}
 
 	query := url.Values{}
 	query.Set("name", options.NewName)