[fx-annotate] Verify invalid tags passed to fx.Annotate #835

annotated.go

@@ -144,6 +144,75 @@ type paramTagsAnnotation struct {
 }
 
 var _ Annotation = paramTagsAnnotation{}
+var (
+	errTagSyntaxSpace            = errors.New(`multiple tags are not separated by space`)
+	errTagKeySyntax              = errors.New("tag key is invalid, Use group, name or optional as tag keys")
+	errTagValueSyntaxQuote       = errors.New(`tag value should start with double quote. i.e. key:"value" `)
+	errTagValueSyntaxEndingQuote = errors.New(`tag value should end in double quote. i.e. key:"value" `)
+)
+
+// Collections of key value pairs within a tag should be separated by a space.
+// Eg: `group:"some" optional:"true"`.
+func verifyTagsSpaceSeparated(tagIdx int, tag string) error {
+	if tagIdx > 0 && tag != "" && tag[0] != ' ' {
+		return errTagSyntaxSpace
+	}
+	return nil
+}
+
+// verify tag values are delimited with double quotes.
+func verifyValueQuote(value string) (string, error) {
+	// starting quote should be a double quote
+	if value[0] != '"' {
+		return "", errTagValueSyntaxQuote
+	}
+	// validate tag value is within quotes
+	i := 1
+	for i < len(value) && value[i] != '"' {
+		if value[i] == '\\' {
+			i++
+		}
+		i++
+	}
+	if i >= len(value) {
+		return "", errTagValueSyntaxEndingQuote
+	}
+	return value[i+1:], nil
+
+}
+
+// Check whether the tag follows valid struct.
+// format and returns an error if it's invalid. (i.e. not following
+// tag:"value" space-separated list )
+// Currently dig accepts only 'name', 'group', 'optional' as valid tag keys.
+func verifyAnnotateTag(tag string) error {
+	tagIdx := 0
+	validKeys := map[string]struct{}{"group": {}, "optional": {}, "name": {}}
+	for ; tag != ""; tagIdx++ {
+		if err := verifyTagsSpaceSeparated(tagIdx, tag); err != nil {
+			return err
+		}
+		i := 0
+		if strings.TrimSpace(tag) == "" {
+			return nil
+		}
+		// parsing the key i.e. till reaching colon :
+		for i < len(tag) && tag[i] != ':' {
+			i++
+		}
+		key := strings.TrimSpace(tag[:i])
+		if _, ok := validKeys[key]; !ok {
+			return errTagKeySyntax
+		}
+		value, err := verifyValueQuote(tag[i+1:])
+		if err != nil {
+			return err
+		}
+		tag = value
+	}
+	return nil
+
+}
 
 // Given func(T1, T2, T3, ..., TN), this generates a type roughly
 // equivalent to,
@@ -159,11 +228,19 @@ var _ Annotation = paramTagsAnnotation{}
 //
 // If there has already been a ParamTag that was applied, this
 // will return an error.
+//
+// If the tag is invalid and has mismatched quotation for example,
+// (`tag_name:"tag_value') , this will return an error.
 
 func (pt paramTagsAnnotation) apply(ann *annotated) error {
 	if len(ann.ParamTags) > 0 {
 		return errors.New("cannot apply more than one line of ParamTags")
 	}
+	for _, tag := range pt.tags {
+		if err := verifyAnnotateTag(tag); err != nil {
+			return err
+		}
+	}
 	ann.ParamTags = pt.tags
 	return nil
 }
@@ -285,10 +362,18 @@ var _ Annotation = resultTagsAnnotation{}
 //
 // If there has already been a ResultTag that was applied, this
 // will return an error.
+//
+// If the tag is invalid and has mismatched quotation for example,
+// (`tag_name:"tag_value') , this will return an error.
 func (rt resultTagsAnnotation) apply(ann *annotated) error {
 	if len(ann.ResultTags) > 0 {
 		return errors.New("cannot apply more than one line of ResultTags")
 	}
+	for _, tag := range rt.tags {
+		if err := verifyAnnotateTag(tag); err != nil {
+			return err
+		}
+	}
 	ann.ResultTags = rt.tags
 	return nil
 }

annotated_test.go

@@ -1474,6 +1474,131 @@ func TestAnnotate(t *testing.T) {
 	})
 }
 
+func TestAnnotateApplyFail(t *testing.T) {
+	type a struct{}
+	type b struct{ a *a }
+	newA := func() *a { return &a{} }
+	newB := func(a *a) *b {
+		return &b{a}
+	}
+
+	var (
+		errTagSyntaxSpace            = `multiple tags are not separated by space`
+		errTagKeySyntax              = "tag key is invalid, Use group, name or optional as tag keys"
+		errTagValueSyntaxQuote       = `tag value should start with double quote. i.e. key:"value" `
+		errTagValueSyntaxEndingQuote = `tag value should end in double quote. i.e. key:"value" `
+	)
+	tests := []struct {
+		give                 string
+		wantErr              string
+		giveAnnotationParam  fx.Annotation
+		giveAnnotationResult fx.Annotation
+	}{
+		{
+			give:                 "Tags value invalid ending quote",
+			wantErr:              errTagValueSyntaxEndingQuote,
+			giveAnnotationParam:  fx.ParamTags(`name:"something'`),
+			giveAnnotationResult: fx.ResultTags(`name:"something'`),
+		},
+		{
+			give:                 "Tags value wrong starting quote",
+			wantErr:              errTagValueSyntaxQuote,
+			giveAnnotationParam:  fx.ParamTags(`name:"something" optional:'true"`),
+			giveAnnotationResult: fx.ResultTags(`name:"something" optional:'true"`),
+		},
+		{
+			give:                 "Tags multiple tags not separated by space",
+			wantErr:              errTagSyntaxSpace,
+			giveAnnotationParam:  fx.ParamTags(`name:"something"group:"something"`),
+			giveAnnotationResult: fx.ResultTags(`name:"something"group:"something"`),
+		},
+		{
+			give:                 "Tags key not equal to group, name or optional",
+			wantErr:              errTagKeySyntax,
+			giveAnnotationParam:  fx.ParamTags(`name1:"something"`),
+			giveAnnotationResult: fx.ResultTags(`name1:"something"`),
+		},
+		{
+			give:                 "Tags key empty",
+			wantErr:              errTagKeySyntax,
+			giveAnnotationParam:  fx.ParamTags(`:"something"`),
+			giveAnnotationResult: fx.ResultTags(`:"something"`),
+		},
+	}
+	for _, tt := range tests {
+		t.Run("Param "+tt.give, func(t *testing.T) {
+			app := NewForTest(t,
+				fx.Provide(
+					fx.Annotate(
+						newA,
+						tt.giveAnnotationParam,
+					),
+				),
+				fx.Invoke(newB),
+			)
+			assert.ErrorContains(t, app.Err(), tt.wantErr)
+		})
+		t.Run("Result "+tt.give, func(t *testing.T) {
+			app := NewForTest(t,
+				fx.Provide(
+					fx.Annotate(
+						newA,
+						tt.giveAnnotationResult,
+					),
+				),
+				fx.Invoke(newB),
+			)
+			assert.ErrorContains(t, app.Err(), tt.wantErr)
+		})
+	}
+}
+
+func TestAnnotateApplySuccess(t *testing.T) {
+	type a struct{}
+	type b struct{ a *a }
+	newA := func() *a { return &a{} }
+	newB := func(a *a) *b {
+		return &b{a}
+	}
+
+	tests := []struct {
+		give                 string
+		giveAnnotationParam  fx.Annotation
+		giveAnnotationResult fx.Annotation
+	}{
+		{
+			give:                 "ParamTags Tag Empty",
+			giveAnnotationParam:  fx.ParamTags(`  `),
+			giveAnnotationResult: fx.ResultTags(`  `),
+		},
+		{
+			give:                 "ParamTags Tag Empty with extra spaces",
+			giveAnnotationParam:  fx.ParamTags(`name:"versionNum"`, `  `),
+			giveAnnotationResult: fx.ResultTags(`   `, `group:"versionNum"`),
+		},
+		{
+			give:                 "ParamTags Tag with \\ ",
+			giveAnnotationParam:  fx.ParamTags(`name:"version\\Num"`, `  `),
+			giveAnnotationResult: fx.ResultTags(``, `group:"version\\Num"`),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.give, func(t *testing.T) {
+			app := NewForTest(t,
+				fx.Provide(
+					fx.Annotate(
+						newA,
+						tt.giveAnnotationParam,
+						tt.giveAnnotationResult,
+					),
+				),
+				fx.Invoke(newB),
+			)
+			require.NoError(t, app.Err())
+		})
+	}
+
+}
 func assertApp(
 	t *testing.T,
 	app interface {

module_test.go

@@ -518,8 +518,8 @@ func TestModuleFailures(t *testing.T) {
 		app := NewForTest(t,
 			fx.Module("module",
 				fx.Provide(fx.Annotate(newA,
-					fx.ParamTags(`"name:"A1"`),
-					fx.ParamTags(`"name:"A2"`),
+					fx.ParamTags(`name:"A1"`),
+					fx.ParamTags(`name:"A2"`),
 				)),
 			),
 		)