vcsim: Fix RetrieveProperties path validation to avoid panic

Closes: #2953
Signed-off-by: syuparn <[email protected]>

Author: Syuparn

simulator/property_collector.go

@@ -52,6 +52,7 @@ func NewPropertyCollector(ref types.ManagedObjectReference) object.Reference {
 
 var errMissingField = errors.New("missing field")
 var errEmptyField = errors.New("empty field")
+var errInvalidField = errors.New("invalid field")
 
 func getObject(ctx *Context, ref types.ManagedObjectReference) (reflect.Value, bool) {
 	var obj mo.Reference
@@ -180,6 +181,11 @@ func fieldValue(rval reflect.Value, p string) (interface{}, error) {
 			rval = rval.Elem()
 		}
 
+		if kind == reflect.Slice {
+			// field of array field cannot be specified
+			return nil, errInvalidField
+		}
+
 		x := ucFirst(name)
 		val := rval.FieldByName(x)
 		if !val.IsValid() {
@@ -316,6 +322,13 @@ func (rr *retrieveResult) collectFields(ctx *Context, rval reflect.Value, fields
 					Name: name,
 				}},
 			})
+		case errInvalidField:
+			content.MissingSet = append(content.MissingSet, types.MissingProperty{
+				Path: name,
+				Fault: types.LocalizedMethodFault{Fault: &types.InvalidProperty{
+					Name: name,
+				}},
+			})
 		}
 	}
 }

simulator/property_collector_test.go

@@ -31,6 +31,7 @@ import (
 	"github.com/vmware/govmomi/simulator/esx"
 	"github.com/vmware/govmomi/simulator/vpx"
 	"github.com/vmware/govmomi/view"
+	"github.com/vmware/govmomi/vim25"
 	"github.com/vmware/govmomi/vim25/methods"
 	"github.com/vmware/govmomi/vim25/mo"
 	"github.com/vmware/govmomi/vim25/soap"
@@ -846,6 +847,53 @@ func TestPropertyCollectorInvalidSpecName(t *testing.T) {
 	}
 }
 
+func TestPropertyCollectorInvalidProperty(t *testing.T) {
+	tests := []struct {
+		name          string
+		path          string
+		expectedFault types.BaseMethodFault
+	}{
+		{
+			"specify property of array property",
+			"config.hardware.device.key",
+			new(types.InvalidProperty),
+		},
+	}
+
+	for _, test := range tests {
+		test := test // assign to local var since loop var is reused
+
+		t.Run(test.name, func(t *testing.T) {
+			m := ESX()
+
+			Test(func(ctx context.Context, c *vim25.Client) {
+				pc := property.DefaultCollector(c)
+
+				vm := Map.Any("VirtualMachine")
+				vmRef := vm.Reference()
+
+				var vmMo mo.VirtualMachine
+				err := pc.RetrieveOne(ctx, vmRef, []string{test.path}, &vmMo)
+				if err == nil {
+					t.Fatal("expected error")
+				}
+
+				// NOTE: since soap.vimFaultError is not exported, use interface literal for assertion instead
+				fault, ok := err.(interface {
+					Fault() types.BaseMethodFault
+				})
+				if !ok {
+					t.Fatalf("err does not have fault: type: %T", err)
+				}
+
+				if reflect.TypeOf(fault.Fault()) != reflect.TypeOf(test.expectedFault) {
+					t.Errorf("expected fault: %T, actual fault: %T", test.expectedFault, fault.Fault())
+				}
+			}, m)
+		})
+	}
+}
+
 func TestPropertyCollectorRecursiveSelectSet(t *testing.T) {
 	ctx := context.Background()