From a6ad58d4a6f74eb475a75931a741db7de65170ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 6 Jul 2022 03:43:45 +0200 Subject: [PATCH] Move signature/json.go to signature/internal/json.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We are going to need it in signature/internal . Signed-off-by: Miloslav Trmač --- signature/{ => internal}/json.go | 36 +++++++++++++-------------- signature/{ => internal}/json_test.go | 30 ++++++---------------- signature/policy_config.go | 31 ++++++++++++----------- signature/policy_config_test.go | 14 +++++++++++ signature/simple.go | 16 ++++++------ 5 files changed, 64 insertions(+), 63 deletions(-) rename signature/{ => internal}/json.go (64%) rename signature/{ => internal}/json_test.go (77%) diff --git a/signature/json.go b/signature/internal/json.go similarity index 64% rename from signature/json.go rename to signature/internal/json.go index 9e592863da..0f39fe0ad2 100644 --- a/signature/json.go +++ b/signature/internal/json.go @@ -1,4 +1,4 @@ -package signature +package internal import ( "bytes" @@ -7,34 +7,34 @@ import ( "io" ) -// jsonFormatError is returned when JSON does not match expected format. -type jsonFormatError string +// JSONFormatError is returned when JSON does not match expected format. +type JSONFormatError string -func (err jsonFormatError) Error() string { +func (err JSONFormatError) Error() string { return string(err) } -// paranoidUnmarshalJSONObject unmarshals data as a JSON object, but failing on the slightest unexpected aspect +// ParanoidUnmarshalJSONObject unmarshals data as a JSON object, but failing on the slightest unexpected aspect // (including duplicated keys, unrecognized keys, and non-matching types). Uses fieldResolver to // determine the destination for a field value, which should return a pointer to the destination if valid, or nil if the key is rejected. // // The fieldResolver approach is useful for decoding the Policy.Transports map; using it for structs is a bit lazy, // we could use reflection to automate this. Later? -func paranoidUnmarshalJSONObject(data []byte, fieldResolver func(string) interface{}) error { +func ParanoidUnmarshalJSONObject(data []byte, fieldResolver func(string) interface{}) error { seenKeys := map[string]struct{}{} dec := json.NewDecoder(bytes.NewReader(data)) t, err := dec.Token() if err != nil { - return jsonFormatError(err.Error()) + return JSONFormatError(err.Error()) } if t != json.Delim('{') { - return jsonFormatError(fmt.Sprintf("JSON object expected, got \"%s\"", t)) + return JSONFormatError(fmt.Sprintf("JSON object expected, got \"%s\"", t)) } for { t, err := dec.Token() if err != nil { - return jsonFormatError(err.Error()) + return JSONFormatError(err.Error()) } if t == json.Delim('}') { break @@ -43,34 +43,34 @@ func paranoidUnmarshalJSONObject(data []byte, fieldResolver func(string) interfa key, ok := t.(string) if !ok { // Coverage: This should never happen, dec.Token() rejects non-string-literals in this state. - return jsonFormatError(fmt.Sprintf("Key string literal expected, got \"%s\"", t)) + return JSONFormatError(fmt.Sprintf("Key string literal expected, got \"%s\"", t)) } if _, ok := seenKeys[key]; ok { - return jsonFormatError(fmt.Sprintf("Duplicate key \"%s\"", key)) + return JSONFormatError(fmt.Sprintf("Duplicate key \"%s\"", key)) } seenKeys[key] = struct{}{} valuePtr := fieldResolver(key) if valuePtr == nil { - return jsonFormatError(fmt.Sprintf("Unknown key \"%s\"", key)) + return JSONFormatError(fmt.Sprintf("Unknown key \"%s\"", key)) } // This works like json.Unmarshal, in particular it allows us to implement UnmarshalJSON to implement strict parsing of the field value. if err := dec.Decode(valuePtr); err != nil { - return jsonFormatError(err.Error()) + return JSONFormatError(err.Error()) } } if _, err := dec.Token(); err != io.EOF { - return jsonFormatError("Unexpected data after JSON object") + return JSONFormatError("Unexpected data after JSON object") } return nil } -// paranoidUnmarshalJSONObject unmarshals data as a JSON object, but failing on the slightest unexpected aspect +// ParanoidUnmarshalJSONObjectExactFields unmarshals data as a JSON object, but failing on the slightest unexpected aspect // (including duplicated keys, unrecognized keys, and non-matching types). Each of the fields in exactFields // must be present exactly once, and none other fields are accepted. -func paranoidUnmarshalJSONObjectExactFields(data []byte, exactFields map[string]interface{}) error { +func ParanoidUnmarshalJSONObjectExactFields(data []byte, exactFields map[string]interface{}) error { seenKeys := map[string]struct{}{} - if err := paranoidUnmarshalJSONObject(data, func(key string) interface{} { + if err := ParanoidUnmarshalJSONObject(data, func(key string) interface{} { if valuePtr, ok := exactFields[key]; ok { seenKeys[key] = struct{}{} return valuePtr @@ -81,7 +81,7 @@ func paranoidUnmarshalJSONObjectExactFields(data []byte, exactFields map[string] } for key := range exactFields { if _, ok := seenKeys[key]; !ok { - return jsonFormatError(fmt.Sprintf(`Key "%s" missing in a JSON object`, key)) + return JSONFormatError(fmt.Sprintf(`Key "%s" missing in a JSON object`, key)) } } return nil diff --git a/signature/json_test.go b/signature/internal/json_test.go similarity index 77% rename from signature/json_test.go rename to signature/internal/json_test.go index f62c1994a0..bb408333fb 100644 --- a/signature/json_test.go +++ b/signature/internal/json_test.go @@ -1,4 +1,4 @@ -package signature +package internal import ( "encoding/json" @@ -8,20 +8,6 @@ import ( "github.com/stretchr/testify/require" ) -type mSI map[string]interface{} // To minimize typing the long name - -// A short-hand way to get a JSON object field value or panic. No error handling done, we know -// what we are working with, a panic in a test is good enough, and fitting test cases on a single line -// is a priority. -func x(m mSI, fields ...string) mSI { - for _, field := range fields { - // Not .(mSI) because type assertion of an unnamed type to a named type always fails (the types - // are not "identical"), but the assignment is fine because they are "assignable". - m = m[field].(map[string]interface{}) - } - return m -} - // implementsUnmarshalJSON is a minimalistic type used to detect that // paranoidUnmarshalJSONObject uses the json.Unmarshaler interface of resolved // pointers. @@ -58,20 +44,20 @@ func TestParanoidUnmarshalJSONObject(t *testing.T) { // Empty object ts = testStruct{} - err := paranoidUnmarshalJSONObject([]byte(`{}`), tsResolver) + err := ParanoidUnmarshalJSONObject([]byte(`{}`), tsResolver) require.NoError(t, err) assert.Equal(t, testStruct{}, ts) // Success ts = testStruct{} - err = paranoidUnmarshalJSONObject([]byte(`{"a":"x", "b":2}`), tsResolver) + err = ParanoidUnmarshalJSONObject([]byte(`{"a":"x", "b":2}`), tsResolver) require.NoError(t, err) assert.Equal(t, testStruct{A: "x", B: 2}, ts) // json.Unmarshaler is used for decoding values ts = testStruct{} unmarshalJSONCalled = implementsUnmarshalJSON(false) - err = paranoidUnmarshalJSONObject([]byte(`{"implementsUnmarshalJSON":true}`), tsResolver) + err = ParanoidUnmarshalJSONObject([]byte(`{"implementsUnmarshalJSON":true}`), tsResolver) require.NoError(t, err) assert.Equal(t, unmarshalJSONCalled, implementsUnmarshalJSON(true)) @@ -89,7 +75,7 @@ func TestParanoidUnmarshalJSONObject(t *testing.T) { `{"a":"value"}{}`, // Extra data after object } { ts = testStruct{} - err := paranoidUnmarshalJSONObject([]byte(input), tsResolver) + err := ParanoidUnmarshalJSONObject([]byte(input), tsResolver) assert.Error(t, err, input) } } @@ -107,11 +93,11 @@ func TestParanoidUnmarshalJSONObjectExactFields(t *testing.T) { } // Empty object - err := paranoidUnmarshalJSONObjectExactFields([]byte(`{}`), map[string]interface{}{}) + err := ParanoidUnmarshalJSONObjectExactFields([]byte(`{}`), map[string]interface{}{}) require.NoError(t, err) // Success - err = paranoidUnmarshalJSONObjectExactFields([]byte(`{"string": "a", "float64": 3.5, "raw": {"a":"b"}, "unmarshaller": true}`), exactFields) + err = ParanoidUnmarshalJSONObjectExactFields([]byte(`{"string": "a", "float64": 3.5, "raw": {"a":"b"}, "unmarshaller": true}`), exactFields) require.NoError(t, err) assert.Equal(t, "a", stringValue) assert.Equal(t, 3.5, float64Value) @@ -131,7 +117,7 @@ func TestParanoidUnmarshalJSONObjectExactFields(t *testing.T) { `{"string": 1, "float64": 3.5, "raw": {"a":"b"}, "unmarshaller": true}`, // Type mismatch `{"string": "a", "float64": 3.5, "raw": {"a":"b"}, "unmarshaller": true}{}`, // Extra data after object } { - err := paranoidUnmarshalJSONObjectExactFields([]byte(input), exactFields) + err := ParanoidUnmarshalJSONObjectExactFields([]byte(input), exactFields) assert.Error(t, err, input) } } diff --git a/signature/policy_config.go b/signature/policy_config.go index 00b94128db..51d4da275f 100644 --- a/signature/policy_config.go +++ b/signature/policy_config.go @@ -22,6 +22,7 @@ import ( "regexp" "github.com/containers/image/v5/docker/reference" + "github.com/containers/image/v5/signature/internal" "github.com/containers/image/v5/transports" "github.com/containers/image/v5/types" "github.com/containers/storage/pkg/homedir" @@ -104,7 +105,7 @@ var _ json.Unmarshaler = (*Policy)(nil) func (p *Policy) UnmarshalJSON(data []byte) error { *p = Policy{} transports := policyTransportsMap{} - if err := paranoidUnmarshalJSONObject(data, func(key string) interface{} { + if err := internal.ParanoidUnmarshalJSONObject(data, func(key string) interface{} { switch key { case "default": return &p.Default @@ -135,10 +136,10 @@ func (m *policyTransportsMap) UnmarshalJSON(data []byte) error { // We can't unmarshal directly into map values because it is not possible to take an address of a map value. // So, use a temporary map of pointers-to-slices and convert. tmpMap := map[string]*PolicyTransportScopes{} - if err := paranoidUnmarshalJSONObject(data, func(key string) interface{} { + if err := internal.ParanoidUnmarshalJSONObject(data, func(key string) interface{} { // transport can be nil transport := transports.Get(key) - // paranoidUnmarshalJSONObject detects key duplication for us, check just to be safe. + // internal.ParanoidUnmarshalJSONObject detects key duplication for us, check just to be safe. if _, ok := tmpMap[key]; ok { return nil } @@ -181,8 +182,8 @@ func (m *policyTransportScopesWithTransport) UnmarshalJSON(data []byte) error { // We can't unmarshal directly into map values because it is not possible to take an address of a map value. // So, use a temporary map of pointers-to-slices and convert. tmpMap := map[string]*PolicyRequirements{} - if err := paranoidUnmarshalJSONObject(data, func(key string) interface{} { - // paranoidUnmarshalJSONObject detects key duplication for us, check just to be safe. + if err := internal.ParanoidUnmarshalJSONObject(data, func(key string) interface{} { + // internal.ParanoidUnmarshalJSONObject detects key duplication for us, check just to be safe. if _, ok := tmpMap[key]; ok { return nil } @@ -269,7 +270,7 @@ var _ json.Unmarshaler = (*prInsecureAcceptAnything)(nil) func (pr *prInsecureAcceptAnything) UnmarshalJSON(data []byte) error { *pr = prInsecureAcceptAnything{} var tmp prInsecureAcceptAnything - if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + if err := internal.ParanoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ "type": &tmp.Type, }); err != nil { return err @@ -299,7 +300,7 @@ var _ json.Unmarshaler = (*prReject)(nil) func (pr *prReject) UnmarshalJSON(data []byte) error { *pr = prReject{} var tmp prReject - if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + if err := internal.ParanoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ "type": &tmp.Type, }); err != nil { return err @@ -361,7 +362,7 @@ func (pr *prSignedBy) UnmarshalJSON(data []byte) error { var tmp prSignedBy var gotKeyPath, gotKeyData = false, false var signedIdentity json.RawMessage - if err := paranoidUnmarshalJSONObject(data, func(key string) interface{} { + if err := internal.ParanoidUnmarshalJSONObject(data, func(key string) interface{} { switch key { case "type": return &tmp.Type @@ -469,7 +470,7 @@ func (pr *prSignedBaseLayer) UnmarshalJSON(data []byte) error { *pr = prSignedBaseLayer{} var tmp prSignedBaseLayer var baseLayerIdentity json.RawMessage - if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + if err := internal.ParanoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ "type": &tmp.Type, "baseLayerIdentity": &baseLayerIdentity, }); err != nil { @@ -538,7 +539,7 @@ var _ json.Unmarshaler = (*prmMatchExact)(nil) func (prm *prmMatchExact) UnmarshalJSON(data []byte) error { *prm = prmMatchExact{} var tmp prmMatchExact - if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + if err := internal.ParanoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ "type": &tmp.Type, }); err != nil { return err @@ -568,7 +569,7 @@ var _ json.Unmarshaler = (*prmMatchRepoDigestOrExact)(nil) func (prm *prmMatchRepoDigestOrExact) UnmarshalJSON(data []byte) error { *prm = prmMatchRepoDigestOrExact{} var tmp prmMatchRepoDigestOrExact - if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + if err := internal.ParanoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ "type": &tmp.Type, }); err != nil { return err @@ -598,7 +599,7 @@ var _ json.Unmarshaler = (*prmMatchRepository)(nil) func (prm *prmMatchRepository) UnmarshalJSON(data []byte) error { *prm = prmMatchRepository{} var tmp prmMatchRepository - if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + if err := internal.ParanoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ "type": &tmp.Type, }); err != nil { return err @@ -638,7 +639,7 @@ var _ json.Unmarshaler = (*prmExactReference)(nil) func (prm *prmExactReference) UnmarshalJSON(data []byte) error { *prm = prmExactReference{} var tmp prmExactReference - if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + if err := internal.ParanoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ "type": &tmp.Type, "dockerReference": &tmp.DockerReference, }); err != nil { @@ -680,7 +681,7 @@ var _ json.Unmarshaler = (*prmExactRepository)(nil) func (prm *prmExactRepository) UnmarshalJSON(data []byte) error { *prm = prmExactRepository{} var tmp prmExactRepository - if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + if err := internal.ParanoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ "type": &tmp.Type, "dockerRepository": &tmp.DockerRepository, }); err != nil { @@ -752,7 +753,7 @@ var _ json.Unmarshaler = (*prmRemapIdentity)(nil) func (prm *prmRemapIdentity) UnmarshalJSON(data []byte) error { *prm = prmRemapIdentity{} var tmp prmRemapIdentity - if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + if err := internal.ParanoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ "type": &tmp.Type, "prefix": &tmp.Prefix, "signedPrefix": &tmp.SignedPrefix, diff --git a/signature/policy_config_test.go b/signature/policy_config_test.go index 449e8093c9..7f82d49623 100644 --- a/signature/policy_config_test.go +++ b/signature/policy_config_test.go @@ -18,6 +18,20 @@ import ( "github.com/stretchr/testify/require" ) +type mSI map[string]interface{} // To minimize typing the long name + +// A short-hand way to get a JSON object field value or panic. No error handling done, we know +// what we are working with, a panic in a test is good enough, and fitting test cases on a single line +// is a priority. +func x(m mSI, fields ...string) mSI { + for _, field := range fields { + // Not .(mSI) because type assertion of an unnamed type to a named type always fails (the types + // are not "identical"), but the assignment is fine because they are "assignable". + m = m[field].(map[string]interface{}) + } + return m +} + // policyFixtureContents is a data structure equal to the contents of "fixtures/policy.json" var policyFixtureContents = &Policy{ Default: PolicyRequirements{NewPRReject()}, diff --git a/signature/simple.go b/signature/simple.go index da54c9f45b..1ca571e5aa 100644 --- a/signature/simple.go +++ b/signature/simple.go @@ -106,18 +106,18 @@ var _ json.Unmarshaler = (*untrustedSignature)(nil) func (s *untrustedSignature) UnmarshalJSON(data []byte) error { err := s.strictUnmarshalJSON(data) if err != nil { - if formatErr, ok := err.(jsonFormatError); ok { + if formatErr, ok := err.(internal.JSONFormatError); ok { err = internal.NewInvalidSignatureError(formatErr.Error()) } } return err } -// strictUnmarshalJSON is UnmarshalJSON, except that it may return the internal jsonFormatError error type. -// Splitting it into a separate function allows us to do the jsonFormatError → InvalidSignatureError in a single place, the caller. +// strictUnmarshalJSON is UnmarshalJSON, except that it may return the internal.JSONFormatError error type. +// Splitting it into a separate function allows us to do the internal.JSONFormatError → InvalidSignatureError in a single place, the caller. func (s *untrustedSignature) strictUnmarshalJSON(data []byte) error { var critical, optional json.RawMessage - if err := paranoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ + if err := internal.ParanoidUnmarshalJSONObjectExactFields(data, map[string]interface{}{ "critical": &critical, "optional": &optional, }); err != nil { @@ -127,7 +127,7 @@ func (s *untrustedSignature) strictUnmarshalJSON(data []byte) error { var creatorID string var timestamp float64 var gotCreatorID, gotTimestamp = false, false - if err := paranoidUnmarshalJSONObject(optional, func(key string) interface{} { + if err := internal.ParanoidUnmarshalJSONObject(optional, func(key string) interface{} { switch key { case "creator": gotCreatorID = true @@ -155,7 +155,7 @@ func (s *untrustedSignature) strictUnmarshalJSON(data []byte) error { var t string var image, identity json.RawMessage - if err := paranoidUnmarshalJSONObjectExactFields(critical, map[string]interface{}{ + if err := internal.ParanoidUnmarshalJSONObjectExactFields(critical, map[string]interface{}{ "type": &t, "image": &image, "identity": &identity, @@ -167,14 +167,14 @@ func (s *untrustedSignature) strictUnmarshalJSON(data []byte) error { } var digestString string - if err := paranoidUnmarshalJSONObjectExactFields(image, map[string]interface{}{ + if err := internal.ParanoidUnmarshalJSONObjectExactFields(image, map[string]interface{}{ "docker-manifest-digest": &digestString, }); err != nil { return err } s.UntrustedDockerManifestDigest = digest.Digest(digestString) - return paranoidUnmarshalJSONObjectExactFields(identity, map[string]interface{}{ + return internal.ParanoidUnmarshalJSONObjectExactFields(identity, map[string]interface{}{ "docker-reference": &s.UntrustedDockerReference, }) }