refactor: address comments

Signed-off-by: Binbin Li <libinbin@microsoft.com>

signature/algorithm.go

@@ -33,7 +33,10 @@ const (
 
 // KeySpec defines a key type and size.
 type KeySpec struct {
+	// KeyType is the type of the key.
 	Type KeyType
+
+	// KeySize is the size of the key in bits.
 	Size int
 }
 
@@ -62,7 +65,7 @@ func ExtractKeySpec(signingCert *x509.Certificate) (KeySpec, error) {
 			}, nil
 		default:
 			return KeySpec{}, &UnsupportedSigningKeyError{
-				Msg: fmt.Sprintf("rsa key size %d is not supported", bitSize),
+				Msg: fmt.Sprintf("rsa key size %d bits is not supported", bitSize),
 			}
 		}
 	case *ecdsa.PublicKey:
@@ -74,12 +77,12 @@ func ExtractKeySpec(signingCert *x509.Certificate) (KeySpec, error) {
 			}, nil
 		default:
 			return KeySpec{}, &UnsupportedSigningKeyError{
-				Msg: fmt.Sprintf("ecdsa key size %d is not supported", bitSize),
+				Msg: fmt.Sprintf("ecdsa key size %d bits is not supported", bitSize),
 			}
 		}
 	}
 	return KeySpec{}, &UnsupportedSigningKeyError{
-		Msg: "invalid public key type",
+		Msg: "unsupported public key type",
 	}
 }
 

signature/envelope.go

@@ -10,9 +10,12 @@
 // local crypto library or the external signing plugin.
 package signature
 
-import "fmt"
+import (
+	"fmt"
+	"sync"
+)
 
-// Envelope provides functions to basic functions to manipulate signatures.
+// Envelope provides basic functions to manipulate signatures.
 type Envelope interface {
 	// Sign generates and sign the envelope according to the sign request.
 	Sign(req *SignRequest) ([]byte, error)
@@ -41,7 +44,7 @@ type envelopeFunc struct {
 
 // envelopeFuncs maps envelope media type to corresponding constructors and
 // parsers.
-var envelopeFuncs map[string]envelopeFunc
+var envelopeFuncs sync.Map
 
 // RegisterEnvelopeType registers newFunc and parseFunc for the given mediaType.
 // Those functions are intended to be called when creating a new envelope.
@@ -50,43 +53,40 @@ func RegisterEnvelopeType(mediaType string, newFunc NewEnvelopeFunc, parseFunc P
 	if newFunc == nil || parseFunc == nil {
 		return fmt.Errorf("required functions not provided")
 	}
-	if envelopeFuncs == nil {
-		envelopeFuncs = make(map[string]envelopeFunc)
-	}
-
-	envelopeFuncs[mediaType] = envelopeFunc{
+	envelopeFuncs.Store(mediaType, envelopeFunc{
 		newFunc:   newFunc,
 		parseFunc: parseFunc,
-	}
+	})
 	return nil
 }
 
 // RegisteredEnvelopeTypes lists registered envelope media types.
 func RegisteredEnvelopeTypes() []string {
 	var types []string
 
-	for envelopeType := range envelopeFuncs {
-		types = append(types, envelopeType)
-	}
+	envelopeFuncs.Range(func(k, v interface{}) bool {
+		types = append(types, k.(string))
+		return true
+	})
 
 	return types
 }
 
 // NewEnvelope generates an envelope of given media type.
 func NewEnvelope(mediaType string) (Envelope, error) {
-	envelopeFunc, ok := envelopeFuncs[mediaType]
+	val, ok := envelopeFuncs.Load(mediaType)
 	if !ok {
 		return nil, &UnsupportedSignatureFormatError{MediaType: mediaType}
 	}
-	return envelopeFunc.newFunc(), nil
+	return val.(envelopeFunc).newFunc(), nil
 }
 
-// ParseEnvelope generates an envelope by given envelope bytes with specified
+// ParseEnvelope generates an envelope for given envelope bytes with specified
 // media type.
 func ParseEnvelope(mediaType string, envelopeBytes []byte) (Envelope, error) {
-	envelopeFunc, ok := envelopeFuncs[mediaType]
+	val, ok := envelopeFuncs.Load(mediaType)
 	if !ok {
 		return nil, &UnsupportedSignatureFormatError{MediaType: mediaType}
 	}
-	return envelopeFunc.parseFunc(envelopeBytes)
+	return val.(envelopeFunc).parseFunc(envelopeBytes)
 }

signature/envelope_test.go

@@ -2,9 +2,22 @@ package signature
 
 import (
 	"reflect"
+	"sync"
 	"testing"
 )
 
+var (
+	emptyFuncs sync.Map
+	validFuncs sync.Map
+)
+
+func init() {
+	validFuncs.Store(testMediaType, envelopeFunc{
+		newFunc: testNewFunc,
+		parseFunc: testParseFunc,
+	})
+}
+
 // mock an envelope that implements signature.Envelope.
 type testEnvelope struct {
 }
@@ -78,19 +91,17 @@ func TestRegisterEnvelopeType(t *testing.T) {
 func TestRegisteredEnvelopeTypes(t *testing.T) {
 	tests := []struct {
 		name          string
-		envelopeFuncs map[string]envelopeFunc
+		envelopeFuncs sync.Map
 		expect        []string
 	}{
 		{
 			name:          "empty map",
-			envelopeFuncs: make(map[string]envelopeFunc),
+			envelopeFuncs: emptyFuncs,
 			expect:        nil,
 		},
 		{
 			name: "nonempty map",
-			envelopeFuncs: map[string]envelopeFunc{
-				testMediaType: {},
-			},
+			envelopeFuncs: validFuncs,
 			expect: []string{testMediaType},
 		},
 	}
@@ -111,25 +122,21 @@ func TestNewEnvelope(t *testing.T) {
 	tests := []struct {
 		name          string
 		mediaType     string
-		envelopeFuncs map[string]envelopeFunc
+		envelopeFuncs sync.Map
 		expect        Envelope
 		expectErr     bool
 	}{
 		{
 			name:          "unsupported media type",
 			mediaType:     testMediaType,
-			envelopeFuncs: make(map[string]envelopeFunc),
+			envelopeFuncs: emptyFuncs,
 			expect:        nil,
 			expectErr:     true,
 		},
 		{
 			name:      "valid media type",
 			mediaType: testMediaType,
-			envelopeFuncs: map[string]envelopeFunc{
-				testMediaType: {
-					newFunc: testNewFunc,
-				},
-			},
+			envelopeFuncs: validFuncs,
 			expect:    testEnvelope{},
 			expectErr: false,
 		},
@@ -154,25 +161,21 @@ func TestParseEnvelope(t *testing.T) {
 	tests := []struct {
 		name          string
 		mediaType     string
-		envelopeFuncs map[string]envelopeFunc
+		envelopeFuncs sync.Map
 		expect        Envelope
 		expectErr     bool
 	}{
 		{
 			name:          "unsupported media type",
 			mediaType:     testMediaType,
-			envelopeFuncs: make(map[string]envelopeFunc),
+			envelopeFuncs: emptyFuncs,
 			expect:        nil,
 			expectErr:     true,
 		},
 		{
 			name:      "valid media type",
 			mediaType: testMediaType,
-			envelopeFuncs: map[string]envelopeFunc{
-				testMediaType: {
-					parseFunc: testParseFunc,
-				},
-			},
+			envelopeFuncs: validFuncs,
 			expect:    testEnvelope{},
 			expectErr: false,
 		},

signature/errors.go

@@ -18,17 +18,17 @@ func (e *SignatureIntegrityError) Unwrap() error {
 	return e.Err
 }
 
-// MalformedSignatureError is used when Signature envelope is malformed.
-type MalformedSignatureError struct {
+// InvalidSignatureError is used when Signature envelope is invalid.
+type InvalidSignatureError struct {
 	Msg string
 }
 
 // Error returns the error message or the default message if not provided.
-func (e MalformedSignatureError) Error() string {
+func (e InvalidSignatureError) Error() string {
 	if e.Msg != "" {
 		return e.Msg
 	}
-	return "signature envelope format is malformed"
+	return "signature envelope format is invalid"
 
 }
 
@@ -71,45 +71,45 @@ func (e UnsupportedSigningKeyError) Error() string {
 	return "signing key is not supported"
 }
 
-// MalformedArgumentError is used when an argument to a function is malformed.
-type MalformedArgumentError struct {
+// InvalidArgumentError is used when an argument to a function is invalid.
+type InvalidArgumentError struct {
 	Param string
 	Err   error
 }
 
 // Error returns the error message.
-func (e *MalformedArgumentError) Error() string {
+func (e *InvalidArgumentError) Error() string {
 	if e.Err != nil {
-		return fmt.Sprintf("%q param is malformed. Error: %s", e.Param, e.Err.Error())
+		return fmt.Sprintf("%q param is invalid. Error: %s", e.Param, e.Err.Error())
 	}
-	return fmt.Sprintf("%q param is malformed", e.Param)
+	return fmt.Sprintf("%q param is invalid", e.Param)
 }
 
 // Unwrap returns the unwrapped error
-func (e *MalformedArgumentError) Unwrap() error {
+func (e *InvalidArgumentError) Unwrap() error {
 	return e.Err
 }
 
-// MalformedSignRequestError is used when SignRequest is malformed.
-type MalformedSignRequestError struct {
+// InvalidSignRequestError is used when SignRequest is invalid.
+type InvalidSignRequestError struct {
 	Msg string
 }
 
 // Error returns the error message or the default message if not provided.
-func (e *MalformedSignRequestError) Error() string {
+func (e *InvalidSignRequestError) Error() string {
 	if e.Msg != "" {
 		return e.Msg
 	}
-	return "SignRequest is malformed"
+	return "SignRequest is invalid"
 }
 
-// SignatureAlgoNotSupportedError is used when signing algo is not supported.
-type SignatureAlgoNotSupportedError struct {
+// UnsupportedSignatureAlgoError is used when signing algo is not supported.
+type UnsupportedSignatureAlgoError struct {
 	Alg string
 }
 
 // Error returns the formatted error message.
-func (e *SignatureAlgoNotSupportedError) Error() string {
+func (e *UnsupportedSignatureAlgoError) Error() string {
 	return fmt.Sprintf("signature algorithm %q is not supported", e.Alg)
 }
 
@@ -121,12 +121,12 @@ func (e *SignatureEnvelopeNotFoundError) Error() string {
 	return "signature envelope is not present"
 }
 
-// EnvelopeKeyRepeatedError is used when repeated key name found in the envelope.
-type EnvelopeKeyRepeatedError struct {
+// DuplicateKeyError is used when repeated key name found.
+type DuplicateKeyError struct {
 	Key string
 }
 
 // Error returns the formatted error message.
-func (e *EnvelopeKeyRepeatedError) Error() string {
-	return fmt.Sprintf("repeated key: %q exists in the envelope.", e.Key)
+func (e *DuplicateKeyError) Error() string {
+	return fmt.Sprintf("repeated key: %q exists.", e.Key)
 }