Move strict JSON parsing utilities into a separate file.

No semantic change, only a reorganization: The utilities now return
jsonFormatError instead of InvalidSignatureError, but their only
caller maps it back.

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
+}

signature/json_test.go

@@ -0,0 +1,64 @@
+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
+
+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)
+}

signature/signature.go

@@ -65,52 +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

signature/signature_test.go

@@ -39,62 +39,6 @@ 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.