diff --git a/signature/json.go b/signature/json.go new file mode 100644 index 00000000..55915c84 --- /dev/null +++ b/signature/json.go @@ -0,0 +1,51 @@ +package signature + +import "fmt" + +// jsonFormatError is returned when JSON does not match expected format. +type jsonFormatError string + +func (err jsonFormatError) Error() string { + return string(err) +} + +// validateExactMapKeys returns an error if the keys of m are not exactly expectedKeys, which must be pairwise distinct +func validateExactMapKeys(m map[string]interface{}, expectedKeys ...string) error { + if len(m) != len(expectedKeys) { + return jsonFormatError("Unexpected keys in a JSON object") + } + + for _, k := range expectedKeys { + if _, ok := m[k]; !ok { + return jsonFormatError(fmt.Sprintf("Key %s missing in a JSON object", k)) + } + } + // Assuming expectedKeys are pairwise distinct, we know m contains len(expectedKeys) different values in expectedKeys. + return nil +} + +// mapField returns a member fieldName of m, if it is a JSON map, or an error. +func mapField(m map[string]interface{}, fieldName string) (map[string]interface{}, error) { + untyped, ok := m[fieldName] + if !ok { + return nil, jsonFormatError(fmt.Sprintf("Field %s missing", fieldName)) + } + v, ok := untyped.(map[string]interface{}) + if !ok { + return nil, jsonFormatError(fmt.Sprintf("Field %s is not a JSON object", fieldName)) + } + return v, nil +} + +// stringField returns a member fieldName of m, if it is a string, or an error. +func stringField(m map[string]interface{}, fieldName string) (string, error) { + untyped, ok := m[fieldName] + if !ok { + return "", jsonFormatError(fmt.Sprintf("Field %s missing", fieldName)) + } + v, ok := untyped.(string) + if !ok { + return "", jsonFormatError(fmt.Sprintf("Field %s is not a JSON object", fieldName)) + } + return v, nil +} diff --git a/signature/json_test.go b/signature/json_test.go new file mode 100644 index 00000000..1c79b268 --- /dev/null +++ b/signature/json_test.go @@ -0,0 +1,76 @@ +package signature + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "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 +} + +func TestValidateExactMapKeys(t *testing.T) { + // Empty map and keys + err := validateExactMapKeys(mSI{}) + assert.NoError(t, err) + + // Success + err = validateExactMapKeys(mSI{"a": nil, "b": 1}, "b", "a") + assert.NoError(t, err) + + // Extra map keys + err = validateExactMapKeys(mSI{"a": nil, "b": 1}, "a") + assert.Error(t, err) + + // Extra expected keys + err = validateExactMapKeys(mSI{"a": 1}, "b", "a") + assert.Error(t, err) + + // Unexpected key values + err = validateExactMapKeys(mSI{"a": 1}, "b") + assert.Error(t, err) +} + +func TestMapField(t *testing.T) { + // Field not found + _, err := mapField(mSI{"a": mSI{}}, "b") + assert.Error(t, err) + + // Field has a wrong type + _, err = mapField(mSI{"a": 1}, "a") + assert.Error(t, err) + + // Success + // FIXME? We can't use mSI as the type of child, that type apparently can't be converted to the raw map type. + child := map[string]interface{}{"b": mSI{}} + m, err := mapField(mSI{"a": child, "b": nil}, "a") + require.NoError(t, err) + assert.Equal(t, child, m) +} + +func TestStringField(t *testing.T) { + // Field not found + _, err := stringField(mSI{"a": "x"}, "b") + assert.Error(t, err) + + // Field has a wrong type + _, err = stringField(mSI{"a": 1}, "a") + assert.Error(t, err) + + // Success + s, err := stringField(mSI{"a": "x", "b": nil}, "a") + require.NoError(t, err) + assert.Equal(t, "x", s) +} diff --git a/signature/mechanism.go b/signature/mechanism.go index 02ab0137..715c5a11 100644 --- a/signature/mechanism.go +++ b/signature/mechanism.go @@ -1,3 +1,5 @@ +// Note: Consider the API unstable until the code supports at least three different image formats or transports. + package signature import ( diff --git a/signature/signature.go b/signature/signature.go index d834d019..b59209ac 100644 --- a/signature/signature.go +++ b/signature/signature.go @@ -36,6 +36,9 @@ type privateSignature struct { Signature } +// Compile-time check that privateSignature implements json.Marshaler +var _ json.Marshaler = (*privateSignature)(nil) + // MarshalJSON implements the json.Marshaler interface. func (s privateSignature) MarshalJSON() ([]byte, error) { return s.marshalJSONWithVariables(time.Now().UTC().Unix(), signatureCreatorID) @@ -62,49 +65,23 @@ func (s privateSignature) marshalJSONWithVariables(timestamp int64, creatorID st return json.Marshal(signature) } -// validateExactMapKeys returns an error if the keys of m are not exactly expectedKeys, which must be pairwise distinct -func validateExactMapKeys(m map[string]interface{}, expectedKeys ...string) error { - if len(m) != len(expectedKeys) { - return InvalidSignatureError{msg: "Unexpected keys in a JSON object"} - } - - for _, k := range expectedKeys { - if _, ok := m[k]; !ok { - return InvalidSignatureError{msg: fmt.Sprintf("Key %s missing in a JSON object", k)} - } - } - // Assuming expectedKeys are pairwise distinct, we know m contains len(expectedKeys) different values in expectedKeys. - return nil -} - -// mapField returns a member fieldName of m, if it is a JSON map, or an error. -func mapField(m map[string]interface{}, fieldName string) (map[string]interface{}, error) { - untyped, ok := m[fieldName] - if !ok { - return nil, InvalidSignatureError{msg: fmt.Sprintf("Field %s missing", fieldName)} - } - v, ok := untyped.(map[string]interface{}) - if !ok { - return nil, InvalidSignatureError{msg: fmt.Sprintf("Field %s is not a JSON object", fieldName)} - } - return v, nil -} - -// stringField returns a member fieldName of m, if it is a string, or an error. -func stringField(m map[string]interface{}, fieldName string) (string, error) { - untyped, ok := m[fieldName] - if !ok { - return "", InvalidSignatureError{msg: fmt.Sprintf("Field %s missing", fieldName)} - } - v, ok := untyped.(string) - if !ok { - return "", InvalidSignatureError{msg: fmt.Sprintf("Field %s is not a JSON object", fieldName)} - } - return v, nil -} +// Compile-time check that privateSignature implements json.Unmarshaler +var _ json.Unmarshaler = (*privateSignature)(nil) // UnmarshalJSON implements the json.Unmarshaler interface func (s *privateSignature) UnmarshalJSON(data []byte) error { + err := s.strictUnmarshalJSON(data) + if err != nil { + if _, ok := err.(jsonFormatError); ok { + err = InvalidSignatureError{msg: err.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. +func (s *privateSignature) strictUnmarshalJSON(data []byte) error { var untyped interface{} if err := json.Unmarshal(data, &untyped); err != nil { return err diff --git a/signature/signature_test.go b/signature/signature_test.go index 5bb7418f..fd258548 100644 --- a/signature/signature_test.go +++ b/signature/signature_test.go @@ -39,76 +39,8 @@ func TestMarshalJSON(t *testing.T) { assert.NoError(t, err) } -type mSI map[string]interface{} // To minimize typing the long name - -func TestValidateExactMapKeys(t *testing.T) { - // Empty map and keys - err := validateExactMapKeys(mSI{}) - assert.NoError(t, err) - - // Success - err = validateExactMapKeys(mSI{"a": nil, "b": 1}, "b", "a") - assert.NoError(t, err) - - // Extra map keys - err = validateExactMapKeys(mSI{"a": nil, "b": 1}, "a") - assert.Error(t, err) - - // Extra expected keys - err = validateExactMapKeys(mSI{"a": 1}, "b", "a") - assert.Error(t, err) - - // Unexpected key values - err = validateExactMapKeys(mSI{"a": 1}, "b") - assert.Error(t, err) -} - -func TestMapField(t *testing.T) { - // Field not found - _, err := mapField(mSI{"a": mSI{}}, "b") - assert.Error(t, err) - - // Field has a wrong type - _, err = mapField(mSI{"a": 1}, "a") - assert.Error(t, err) - - // Success - // FIXME? We can't use mSI as the type of child, that type apparently can't be converted to the raw map type. - child := map[string]interface{}{"b": mSI{}} - m, err := mapField(mSI{"a": child, "b": nil}, "a") - require.NoError(t, err) - assert.Equal(t, child, m) -} - -func TestStringField(t *testing.T) { - // Field not found - _, err := stringField(mSI{"a": "x"}, "b") - assert.Error(t, err) - - // Field has a wrong type - _, err = stringField(mSI{"a": 1}, "a") - assert.Error(t, err) - - // Success - s, err := stringField(mSI{"a": "x", "b": nil}, "a") - require.NoError(t, err) - assert.Equal(t, "x", s) -} - -// 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 -} - // Return the result of modifying validJSON with fn and unmarshaling it into *sig -func tryUnmarshalModified(t *testing.T, sig *privateSignature, validJSON []byte, modifyFn func(mSI)) error { +func tryUnmarshalModifiedSignature(t *testing.T, sig *privateSignature, validJSON []byte, modifyFn func(mSI)) error { var tmp mSI err := json.Unmarshal(validJSON, &tmp) require.NoError(t, err) @@ -118,6 +50,7 @@ func tryUnmarshalModified(t *testing.T, sig *privateSignature, validJSON []byte, testJSON, err := json.Marshal(tmp) require.NoError(t, err) + *sig = privateSignature{} return json.Unmarshal(testJSON, sig) } @@ -184,7 +117,7 @@ func TestUnmarshalJSON(t *testing.T) { func(v mSI) { x(v, "critical", "identity")["docker-reference"] = 1 }, } for _, fn := range breakFns { - err = tryUnmarshalModified(t, &s, validJSON, fn) + err = tryUnmarshalModifiedSignature(t, &s, validJSON, fn) assert.Error(t, err) } @@ -196,17 +129,12 @@ func TestUnmarshalJSON(t *testing.T) { func(v mSI) { delete(x(v, "optional"), "creator") }, } for _, fn := range allowedModificationFns { - s = privateSignature{} - err = tryUnmarshalModified(t, &s, validJSON, fn) + err = tryUnmarshalModifiedSignature(t, &s, validJSON, fn) require.NoError(t, err) assert.Equal(t, validSig, s) } } -type savedEnvironment struct { - vars []string -} - func TestSign(t *testing.T) { mech, err := newGPGSigningMechanismInDirectory(testGPGHomeDirectory) require.NoError(t, err)