Skip to content

Commit

Permalink
Move signature/json.go to signature/internal/json.go
Browse files Browse the repository at this point in the history 
We are going to need it in signature/internal .

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
  • Loading branch information
@mtrmac
mtrmac committed Jul 6, 2022
1 parent a3d083f commit a6ad58d
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 63 deletions.
36 changes: 18 additions & 18 deletions signature/json.go → signature/internal/json.go
View file
@@ -1,4 +1,4 @@
package signature
package internal

import (
"bytes"
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
30 changes: 8 additions & 22 deletions signature/json_test.go → signature/internal/json_test.go
View file
@@ -1,4 +1,4 @@
package signature
package internal

import (
"encoding/json"
Expand All @@ -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.
Expand Down Expand Up @@ -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))

Expand All @@ -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)
}
}
Expand All @@ -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)
Expand All @@ -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)
}
}
31 changes: 16 additions & 15 deletions signature/policy_config.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 14 additions & 0 deletions signature/policy_config_test.go
View file
Expand Up @@ -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()},
Expand Down
16 changes: 8 additions & 8 deletions signature/simple.go
View file
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
})
}
Expand Down

0 comments on commit a6ad58d

Please sign in to comment.