Merge pull request #2874 from Syuparn/issue-2873

vcsim: Fix createVM to encode VM name

simulator/folder.go

@@ -200,7 +200,7 @@ func (f *Folder) CreateFolder(ctx *Context, c *types.CreateFolder) soap.HasFault
 	r := &methods.CreateFolderBody{}
 
 	if folderHasChildType(&f.Folder, "Folder") {
-		name := f.escapeSpecialCharacters(c.Name)
+		name := escapeSpecialCharacters(c.Name)
 
 		if obj := ctx.Map.FindByName(name, f.ChildEntity); obj != nil {
 			r.Fault_ = Fault("", &types.DuplicateName{
@@ -228,7 +228,7 @@ func (f *Folder) CreateFolder(ctx *Context, c *types.CreateFolder) soap.HasFault
 	return r
 }
 
-func (f *Folder) escapeSpecialCharacters(name string) string {
+func escapeSpecialCharacters(name string) string {
 	name = strings.ReplaceAll(name, `%`, strings.ToLower(url.QueryEscape(`%`)))
 	name = strings.ReplaceAll(name, `/`, strings.ToLower(url.QueryEscape(`/`)))
 	name = strings.ReplaceAll(name, `\`, strings.ToLower(url.QueryEscape(`\`)))
@@ -367,9 +367,15 @@ func hostsWithDatastore(hosts []types.ManagedObjectReference, path string) []typ
 }
 
 func (c *createVM) Run(task *Task) (types.AnyType, types.BaseMethodFault) {
+	config := &c.req.Config
+	// escape special characters in vm name
+	if config.Name != escapeSpecialCharacters(config.Name) {
+		deepCopy(c.req.Config, config)
+		config.Name = escapeSpecialCharacters(config.Name)
+	}
+
 	vm, err := NewVirtualMachine(c.ctx, c.Folder.Self, &c.req.Config)
 	if err != nil {
-		folderRemoveChild(c.ctx, &c.Folder.Folder, vm)
 		return nil, err
 	}
 

simulator/virtual_machine.go

@@ -166,6 +166,7 @@ func NewVirtualMachine(ctx *Context, parent types.ManagedObjectReference, spec *
 	vm.Summary.OverallStatus = types.ManagedEntityStatusGreen
 	vm.ConfigStatus = types.ManagedEntityStatusGreen
 
+	// put vm in the folder only if no errors occurred
 	f, _ := asFolderMO(folder)
 	folderPutChild(ctx, f, vm)
 

simulator/virtual_machine_test.go

@@ -30,6 +30,7 @@ import (
 	"github.com/vmware/govmomi/property"
 	"github.com/vmware/govmomi/simulator/esx"
 	"github.com/vmware/govmomi/task"
+	"github.com/vmware/govmomi/vim25"
 	"github.com/vmware/govmomi/vim25/types"
 )
 
@@ -264,6 +265,74 @@ func TestCreateVm(t *testing.T) {
 	}
 }
 
+func TestCreateVmWithSpecialCharaters(t *testing.T) {
+	tests := []struct {
+		name     string
+		expected string
+	}{
+		{`/`, `%2f`},
+		{`\`, `%5c`},
+		{`%`, `%25`},
+		// multiple special characters
+		{`%%`, `%25%25`},
+		// slash-separated name
+		{`foo/bar`, `foo%2fbar`},
+	}
+
+	for _, test := range tests {
+		m := ESX()
+
+		Test(func(ctx context.Context, c *vim25.Client) {
+			finder := find.NewFinder(c, false)
+
+			dc, err := finder.DefaultDatacenter(ctx)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			finder.SetDatacenter(dc)
+			folders, err := dc.Folders(ctx)
+			if err != nil {
+				t.Fatal(err)
+			}
+			vmFolder := folders.VmFolder
+
+			ds, err := finder.DefaultDatastore(ctx)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			spec := types.VirtualMachineConfigSpec{
+				Name: test.name,
+				Files: &types.VirtualMachineFileInfo{
+					VmPathName: fmt.Sprintf("[%s]", ds.Name()),
+				},
+			}
+
+			pool := object.NewResourcePool(c, esx.ResourcePool.Self)
+
+			task, err := vmFolder.CreateVM(ctx, spec, pool, nil)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			info, err := task.WaitForResult(ctx, nil)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			vm := object.NewVirtualMachine(c, info.Result.(types.ManagedObjectReference))
+			name, err := vm.ObjectName(ctx)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if name != test.expected {
+				t.Errorf("expected %s, got %s", test.expected, name)
+			}
+		}, m)
+	}
+}
+
 func TestReconfigVmDevice(t *testing.T) {
 	ctx := context.Background()