Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

types: Ensure List/Map/Object/Set Attributes/AttributeTypes/Elements returns cannot mutate underlying data #591

Merged
merged 4 commits into from Dec 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changelog/591.txt
@@ -0,0 +1,15 @@
```release-note:bug
types/basetypes: Prevented value mutation via the `ListValue`, `MapValue`, and `SetValue` type `Elements()` method return
```

```release-note:bug
types/basetypes: Prevented type mutation via the `ObjectType` type `AttributeTypes()` method return
```

```release-note:bug
types/basetypes: Prevented value mutation via the `ObjectValue` type `AttributeTypes()` and `Attributes()` method returns
```

```release-note:bug
resource/schema/planmodifier: Prevented `assignment to entry in nil map` panic for `Object` type plan modifiers
```
152 changes: 152 additions & 0 deletions internal/fwserver/attribute_plan_modification_test.go
Expand Up @@ -7764,6 +7764,158 @@ func TestNestedAttributeObjectPlanModify(t *testing.T) {
),
},
},
"response-planvalue-unknown-to-known": {
object: testschema.NestedAttributeObjectWithPlanModifiers{
PlanModifiers: []planmodifier.Object{
testplanmodifier.Object{
PlanModifyObjectMethod: func(ctx context.Context, req planmodifier.ObjectRequest, resp *planmodifier.ObjectResponse) {
resp.PlanValue = types.ObjectValueMust(
map[string]attr.Type{
"testattr": types.StringType,
},
map[string]attr.Value{
"testattr": types.StringValue("newtestvalue"),
},
)
},
},
},
},
request: planmodifier.ObjectRequest{
Config: tfsdk.Config{
Raw: tftypes.NewValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test": tftypes.Object{AttributeTypes: map[string]tftypes.Type{"testattr": tftypes.String}},
},
},
map[string]tftypes.Value{
"test": tftypes.NewValue(
tftypes.Object{AttributeTypes: map[string]tftypes.Type{"testattr": tftypes.String}},
nil,
),
},
),
Schema: fwSchema,
},
ConfigValue: types.ObjectNull(
map[string]attr.Type{"testattr": types.StringType},
),
Path: path.Root("test"),
PathExpression: path.MatchRoot("test"),
Plan: tfsdk.Plan{
Raw: tftypes.NewValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test": tftypes.Object{AttributeTypes: map[string]tftypes.Type{"testattr": tftypes.String}},
},
},
map[string]tftypes.Value{
"test": tftypes.NewValue(
tftypes.Object{AttributeTypes: map[string]tftypes.Type{"testattr": tftypes.String}},
tftypes.UnknownValue,
),
},
),
Schema: fwSchema,
},
PlanValue: types.ObjectUnknown(
map[string]attr.Type{"testattr": types.StringType},
),
State: testState,
StateValue: fwValue,
},
response: &ModifyAttributePlanResponse{
AttributePlan: types.ObjectUnknown(
map[string]attr.Type{"testattr": types.StringType},
),
},
expected: &ModifyAttributePlanResponse{
AttributePlan: types.ObjectValueMust(
map[string]attr.Type{
"testattr": types.StringType,
},
map[string]attr.Value{
"testattr": types.StringValue("newtestvalue"),
},
),
},
},
"response-planvalue-unknown-to-known-nested": {
object: testschema.NestedAttributeObject{
Attributes: map[string]fwschema.Attribute{
"testattr": testschema.AttributeWithStringPlanModifiers{
Required: true,
PlanModifiers: []planmodifier.String{
testplanmodifier.String{
PlanModifyStringMethod: func(ctx context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) {
resp.PlanValue = types.StringValue("newtestvalue") // should win over object
},
},
},
},
},
},
request: planmodifier.ObjectRequest{
Config: tfsdk.Config{
Raw: tftypes.NewValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test": tftypes.Object{AttributeTypes: map[string]tftypes.Type{"testattr": tftypes.String}},
},
},
map[string]tftypes.Value{
"test": tftypes.NewValue(
tftypes.Object{AttributeTypes: map[string]tftypes.Type{"testattr": tftypes.String}},
nil,
),
},
),
Schema: fwSchema,
},
ConfigValue: types.ObjectNull(
map[string]attr.Type{"testattr": types.StringType},
),
Path: path.Root("test"),
PathExpression: path.MatchRoot("test"),
Plan: tfsdk.Plan{
Raw: tftypes.NewValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test": tftypes.Object{AttributeTypes: map[string]tftypes.Type{"testattr": tftypes.String}},
},
},
map[string]tftypes.Value{
"test": tftypes.NewValue(
tftypes.Object{AttributeTypes: map[string]tftypes.Type{"testattr": tftypes.String}},
tftypes.UnknownValue,
),
},
),
Schema: fwSchema,
},
PlanValue: types.ObjectUnknown(
map[string]attr.Type{"testattr": types.StringType},
),
State: testState,
StateValue: fwValue,
},
response: &ModifyAttributePlanResponse{
AttributePlan: types.ObjectUnknown(
map[string]attr.Type{"testattr": types.StringType},
),
},
expected: &ModifyAttributePlanResponse{
AttributePlan: types.ObjectValueMust(
map[string]attr.Type{
"testattr": types.StringType,
},
map[string]attr.Value{
"testattr": types.StringValue("newtestvalue"),
},
),
},
},
"response-private": {
object: testschema.NestedAttributeObjectWithPlanModifiers{
PlanModifiers: []planmodifier.Object{
Expand Down
9 changes: 6 additions & 3 deletions types/basetypes/list.go
Expand Up @@ -309,10 +309,13 @@ type ListValue struct {
state attr.ValueState
}

// Elements returns the collection of elements for the List. Returns nil if the
// List is null or unknown.
// Elements returns a copy of the collection of elements for the List.
func (l ListValue) Elements() []attr.Value {
return l.elements
// Ensure callers cannot mutate the internal elements
result := make([]attr.Value, 0, len(l.elements))
result = append(result, l.elements...)

return result
}

// ElementsAs populates `target` with the elements of the ListValue, throwing an
Expand Down
15 changes: 13 additions & 2 deletions types/basetypes/list_test.go
Expand Up @@ -589,11 +589,11 @@ func TestListValueElements(t *testing.T) {
},
"null": {
input: NewListNull(StringType{}),
expected: nil,
expected: []attr.Value{},
},
"unknown": {
input: NewListUnknown(StringType{}),
expected: nil,
expected: []attr.Value{},
},
}

Expand All @@ -612,6 +612,17 @@ func TestListValueElements(t *testing.T) {
}
}

func TestListValueElements_immutable(t *testing.T) {
t.Parallel()

value := NewListValueMust(StringType{}, []attr.Value{NewStringValue("original")})
value.Elements()[0] = NewStringValue("modified")

if !value.Equal(NewListValueMust(StringType{}, []attr.Value{NewStringValue("original")})) {
t.Fatal("unexpected Elements mutation")
}
}

func TestListValueElementType(t *testing.T) {
t.Parallel()

Expand Down
12 changes: 9 additions & 3 deletions types/basetypes/map.go
Expand Up @@ -314,10 +314,16 @@ type MapValue struct {
state attr.ValueState
}

// Elements returns the mapping of elements for the Map. Returns nil if the
// Map is null or unknown.
// Elements returns a copy of the mapping of elements for the Map.
func (m MapValue) Elements() map[string]attr.Value {
return m.elements
// Ensure callers cannot mutate the internal elements
result := make(map[string]attr.Value, len(m.elements))

for key, value := range m.elements {
result[key] = value
}

return result
}

// ElementsAs populates `target` with the elements of the MapValue, throwing an
Expand Down
15 changes: 13 additions & 2 deletions types/basetypes/map_test.go
Expand Up @@ -571,11 +571,11 @@ func TestMapValueElements(t *testing.T) {
},
"null": {
input: NewMapNull(StringType{}),
expected: nil,
expected: map[string]attr.Value{},
},
"unknown": {
input: NewMapUnknown(StringType{}),
expected: nil,
expected: map[string]attr.Value{},
},
}

Expand All @@ -594,6 +594,17 @@ func TestMapValueElements(t *testing.T) {
}
}

func TestMapValueElements_immutable(t *testing.T) {
t.Parallel()

value := NewMapValueMust(StringType{}, map[string]attr.Value{"test": NewStringValue("original")})
value.Elements()["test"] = NewStringValue("modified")

if !value.Equal(NewMapValueMust(StringType{}, map[string]attr.Value{"test": NewStringValue("original")})) {
t.Fatal("unexpected Elements mutation")
}
}

func TestMapValueElementType(t *testing.T) {
t.Parallel()

Expand Down
34 changes: 27 additions & 7 deletions types/basetypes/object.go
Expand Up @@ -50,9 +50,16 @@ func (o ObjectType) WithAttributeTypes(typs map[string]attr.Type) attr.TypeWithA
}
}

// AttributeTypes returns the type's attribute types.
// AttributeTypes returns a copy of the type's attribute types.
func (o ObjectType) AttributeTypes() map[string]attr.Type {
return o.AttrTypes
// Ensure callers cannot mutate the value
result := make(map[string]attr.Type, len(o.AttrTypes))

for key, value := range o.AttrTypes {
result[key] = value
}

return result
}

// TerraformType returns the tftypes.Type that should be used to
Expand Down Expand Up @@ -356,15 +363,28 @@ func (o ObjectValue) As(ctx context.Context, target interface{}, opts ObjectAsOp
}, path.Empty())
}

// Attributes returns the mapping of known attribute values for the Object.
// Returns nil if the Object is null or unknown.
// Attributes returns a copy of the mapping of known attribute values for the Object.
func (o ObjectValue) Attributes() map[string]attr.Value {
return o.attributes
// Ensure callers cannot mutate the internal attributes
result := make(map[string]attr.Value, len(o.attributes))

for name, value := range o.attributes {
result[name] = value
}

return result
}

// AttributeTypes returns the mapping of attribute types for the Object.
// AttributeTypes returns a copy of the mapping of attribute types for the Object.
func (o ObjectValue) AttributeTypes(_ context.Context) map[string]attr.Type {
return o.attributeTypes
// Ensure callers cannot mutate the internal attribute types
result := make(map[string]attr.Type, len(o.attributeTypes))

for name, typ := range o.attributeTypes {
result[name] = typ
}

return result
}

// Type returns an ObjectType with the same attribute types as `o`.
Expand Down