From dda4b0d83888af305063c90eb01c2df20be63502 Mon Sep 17 00:00:00 2001 From: Ben Luddy Date: Thu, 20 Jun 2024 13:35:47 -0400 Subject: [PATCH] Copy input to UnmarshalJSON on the apiextensions JSON types. According to the contract of json.Unmarshaler: "UnmarshalJSON must copy the JSON data if it wishes to retain the data after returning." If the input is not copied, the underlying array of Raw could be reused by the caller of UnmarshalJSON, and any modifications would be visible in Raw (and vice versa). --- .../pkg/apis/apiextensions/v1/marshal.go | 2 +- .../pkg/apis/apiextensions/v1/marshal_test.go | 21 +++++++++++++++++++ .../pkg/apis/apiextensions/v1beta1/marshal.go | 2 +- .../apiextensions/v1beta1/marshal_test.go | 21 +++++++++++++++++++ 4 files changed, 44 insertions(+), 2 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/marshal.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/marshal.go index 12cc2f6f2c9..321bec385c5 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/marshal.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/marshal.go @@ -130,7 +130,7 @@ func (s JSON) MarshalJSON() ([]byte, error) { func (s *JSON) UnmarshalJSON(data []byte) error { if len(data) > 0 && !bytes.Equal(data, nullLiteral) { - s.Raw = data + s.Raw = append(s.Raw[0:0], data...) } return nil } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/marshal_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/marshal_test.go index dae08bd5e3b..4ddab379c5d 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/marshal_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/marshal_test.go @@ -148,3 +148,24 @@ func TestJSONSchemaPropsOrArrayMarshalJSON(t *testing.T) { } } } + +func TestJSONUnderlyingArrayReuse(t *testing.T) { + const want = `{"foo":"bar"}` + + b := []byte(want) + + var s JSON + if err := s.UnmarshalJSON(b); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Underlying array is modified. + copy(b[2:5], "bar") + copy(b[8:11], "foo") + + // If UnmarshalJSON copied the bytes of its argument, then it should not have been affected + // by the mutation. + if got := string(s.Raw); got != want { + t.Errorf("unexpected mutation, got %s want %s", got, want) + } +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/marshal.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/marshal.go index 44941d82eff..43b90387872 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/marshal.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/marshal.go @@ -130,7 +130,7 @@ func (s JSON) MarshalJSON() ([]byte, error) { func (s *JSON) UnmarshalJSON(data []byte) error { if len(data) > 0 && !bytes.Equal(data, nullLiteral) { - s.Raw = data + s.Raw = append(s.Raw[0:0], data...) } return nil } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/marshal_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/marshal_test.go index 99065ff95ad..c45e12f9993 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/marshal_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/marshal_test.go @@ -148,3 +148,24 @@ func TestJSONSchemaPropsOrArrayMarshalJSON(t *testing.T) { } } } + +func TestJSONUnderlyingArrayReuse(t *testing.T) { + const want = `{"foo":"bar"}` + + b := []byte(want) + + var s JSON + if err := s.UnmarshalJSON(b); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Underlying array is modified. + copy(b[2:5], "bar") + copy(b[8:11], "foo") + + // If UnmarshalJSON copied the bytes of its argument, then it should not have been affected + // by the mutation. + if got := string(s.Raw); got != want { + t.Errorf("unexpected mutation, got %s want %s", got, want) + } +}