Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMPROVED] Subject transform validation and error reporting #4202

Merged
merged 2 commits into from Jun 1, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions server/opts.go
Expand Up @@ -2690,7 +2690,7 @@ func parseAccountMappings(v interface{}, acc *Account, errors *[]error, warnings

// Now add them in..
if err := acc.AddWeightedMappings(subj, mappings...); err != nil {
err := &configErr{tk, fmt.Sprintf("Error adding mapping for %q to %q : %v", subj, v.(string), err)}
err := &configErr{tk, fmt.Sprintf("Error adding mapping for %q : %v", subj, err)}
*errors = append(*errors, err)
continue
}
Expand All @@ -2702,7 +2702,7 @@ func parseAccountMappings(v interface{}, acc *Account, errors *[]error, warnings
}
// Now add it in..
if err := acc.AddWeightedMappings(subj, mdest); err != nil {
err := &configErr{tk, fmt.Sprintf("Error adding mapping for %q to %q : %v", subj, v.(string), err)}
err := &configErr{tk, fmt.Sprintf("Error adding mapping for %q : %v", subj, err)}
*errors = append(*errors, err)
continue
}
Expand Down
18 changes: 18 additions & 0 deletions server/subject_transform.go
Expand Up @@ -114,6 +114,12 @@ func NewSubjectTransformWithStrict(src, dest string, strict bool) (*subjectTrans
}
}

if npwcs == 0 {
if tranformType != NoTransform {
return nil, &mappingDestinationErr{token, ErrorMappingDestinationFunctionWildcardIndexOutOfRange}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got to be a shorter version of this no?

ErrorMappingDestinationFunctionWildcardIndexOutOfRange

}
}

if tranformType == NoTransform {
dtokMappingFunctionTypes = append(dtokMappingFunctionTypes, NoTransform)
dtokMappingFunctionTokenIndexes = append(dtokMappingFunctionTokenIndexes, []int{-1})
Expand All @@ -140,6 +146,18 @@ func NewSubjectTransformWithStrict(src, dest string, strict bool) (*subjectTrans
// not all wildcards are being used in the destination
return nil, &mappingDestinationErr{dest, ErrMappingDestinationNotUsingAllWildcards}
}
} else {
// no wildcards used in the source: check that no transform functions are used in the destination
for _, token := range dtokens {
tranformType, _, _, _, err := indexPlaceHolders(token)
if err != nil {
return nil, err
}

if tranformType != NoTransform {
return nil, &mappingDestinationErr{token, ErrorMappingDestinationFunctionWildcardIndexOutOfRange}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

}
}
}

return &subjectTransform{
Expand Down
1 change: 1 addition & 0 deletions server/subject_transform_test.go
Expand Up @@ -149,6 +149,7 @@ func TestSubjectTransforms(t *testing.T) {
shouldErr("foo.*", "foo.{{wildcard(1,2)}}", false) // Too many arguments passed to the mapping function
shouldErr("foo.*", "foo.{{ wildcard5) }}", false) // Bad mapping function
shouldErr("foo.*", "foo.{{splitLeft(2,2}}", false) // arg out of range
shouldErr("foo", "bla.{{wildcard(1)}}", false) // arg out of range with no wildcard in the source

shouldBeOK := func(src, dest string, strict bool) *subjectTransform {
t.Helper()
Expand Down