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.

Sign up for GitHub

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

Jump to bottom

[IMPROVED] Subject transform validation and error reporting #4202

Merged
merged 2 commits into from Jun 1, 2023
Merged

[IMPROVED] Subject transform validation and error reporting #4202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
08987bd
1: Improves error reporting for weighted mappings:
jnmoyne May 31, 2023
0bd3fa4
Harmonize subject mapping error variable names
jnmoyne Jun 1, 2023
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
20 changes: 10 additions & 10 deletions server/errors.go
View file
Open in desktop
Expand Up @@ -208,20 +208,20 @@ var (
// ErrUnknownMappingDestinationFunction is returned when a subject mapping destination contains an unknown mustache-escaped mapping function.
ErrUnknownMappingDestinationFunction = fmt.Errorf("%w: unknown function", ErrInvalidMappingDestination)

// ErrorMappingDestinationFunctionWildcardIndexOutOfRange is returned when the mapping destination function is passed an out of range wildcard index value for one of it's arguments
ErrorMappingDestinationFunctionWildcardIndexOutOfRange = fmt.Errorf("%w: wildcard index out of range", ErrInvalidMappingDestination)
// ErrMappingDestinationIndexOutOfRange is returned when the mapping destination function is passed an out of range wildcard index value for one of it's arguments
ErrMappingDestinationIndexOutOfRange = fmt.Errorf("%w: wildcard index out of range", ErrInvalidMappingDestination)

// ErrorMappingDestinationFunctionNotEnoughArguments is returned when the mapping destination function is not passed enough arguments
ErrorMappingDestinationFunctionNotEnoughArguments = fmt.Errorf("%w: not enough arguments passed to the function", ErrInvalidMappingDestination)
// ErrMappingDestinationNotEnoughArgs is returned when the mapping destination function is not passed enough arguments
ErrMappingDestinationNotEnoughArgs = fmt.Errorf("%w: not enough arguments passed to the function", ErrInvalidMappingDestination)

// ErrorMappingDestinationFunctionInvalidArgument is returned when the mapping destination function is passed and invalid argument
ErrorMappingDestinationFunctionInvalidArgument = fmt.Errorf("%w: function argument is invalid or in the wrong format", ErrInvalidMappingDestination)
// ErrMappingDestinationInvalidArg is returned when the mapping destination function is passed and invalid argument
ErrMappingDestinationInvalidArg = fmt.Errorf("%w: function argument is invalid or in the wrong format", ErrInvalidMappingDestination)

// ErrorMappingDestinationFunctionTooManyArguments is returned when the mapping destination function is passed too many arguments
ErrorMappingDestinationFunctionTooManyArguments = fmt.Errorf("%w: too many arguments passed to the function", ErrInvalidMappingDestination)
// ErrMappingDestinationTooManyArgs is returned when the mapping destination function is passed too many arguments
ErrMappingDestinationTooManyArgs = fmt.Errorf("%w: too many arguments passed to the function", ErrInvalidMappingDestination)

// ErrorMappingDestinationFunctionNotSupportedForImport is returned when you try to use a mapping function other than wildcard in a transform that needs to be reversible (i.e. an import)
ErrorMappingDestinationFunctionNotSupportedForImport = fmt.Errorf("%w: the only mapping function allowed for import transforms is {{Wildcard()}}", ErrInvalidMappingDestination)
// ErrMappingDestinationNotSupportedForImport is returned when you try to use a mapping function other than wildcard in a transform that needs to be reversible (i.e. an import)
ErrMappingDestinationNotSupportedForImport = fmt.Errorf("%w: the only mapping function allowed for import transforms is {{Wildcard()}}", ErrInvalidMappingDestination)
)

// mappingDestinationErr is a type of subject mapping destination error
Expand Down
4 changes: 2 additions & 2 deletions server/opts.go
View file
Open in desktop
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
50 changes: 34 additions & 16 deletions server/subject_transform.go
View file
Open in desktop
Expand Up @@ -110,7 +110,13 @@ func NewSubjectTransformWithStrict(src, dest string, strict bool) (*subjectTrans

if strict {
if tranformType != NoTransform && tranformType != Wildcard {
return nil, &mappingDestinationErr{token, ErrorMappingDestinationFunctionNotSupportedForImport}
return nil, &mappingDestinationErr{token, ErrMappingDestinationNotSupportedForImport}
}
}

if npwcs == 0 {
if tranformType != NoTransform {
return nil, &mappingDestinationErr{token, ErrMappingDestinationIndexOutOfRange}
}
}

Expand All @@ -125,7 +131,7 @@ func NewSubjectTransformWithStrict(src, dest string, strict bool) (*subjectTrans
var stis []int
for _, wildcardIndex := range transformArgWildcardIndexes {
if wildcardIndex > npwcs {
return nil, &mappingDestinationErr{fmt.Sprintf("%s: [%d]", token, wildcardIndex), ErrorMappingDestinationFunctionWildcardIndexOutOfRange}
return nil, &mappingDestinationErr{fmt.Sprintf("%s: [%d]", token, wildcardIndex), ErrMappingDestinationIndexOutOfRange}
}
stis = append(stis, sti[wildcardIndex])
}
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, ErrMappingDestinationIndexOutOfRange}
}
}
}

return &subjectTransform{
Expand Down Expand Up @@ -173,18 +191,18 @@ func getMappingFunctionArgs(functionRegEx *regexp.Regexp, token string) []string
// Helper for mapping functions that take a wildcard index and an integer as arguments
func transformIndexIntArgsHelper(token string, args []string, transformType int16) (int16, []int, int32, string, error) {
if len(args) < 2 {
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrorMappingDestinationFunctionNotEnoughArguments}
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrMappingDestinationNotEnoughArgs}
}
if len(args) > 2 {
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrorMappingDestinationFunctionTooManyArguments}
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrMappingDestinationTooManyArgs}
}
i, err := strconv.Atoi(strings.Trim(args[0], " "))
if err != nil {
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrorMappingDestinationFunctionInvalidArgument}
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrMappingDestinationInvalidArg}
}
mappingFunctionIntArg, err := strconv.Atoi(strings.Trim(args[1], " "))
if err != nil {
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrorMappingDestinationFunctionInvalidArgument}
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrMappingDestinationInvalidArg}
}

return transformType, []int{i}, int32(mappingFunctionIntArg), _EMPTY_, nil
Expand Down Expand Up @@ -212,36 +230,36 @@ func indexPlaceHolders(token string) (int16, []int, int32, string, error) {
args := getMappingFunctionArgs(wildcardMappingFunctionRegEx, token)
if args != nil {
if len(args) == 1 && args[0] == _EMPTY_ {
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrorMappingDestinationFunctionNotEnoughArguments}
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrMappingDestinationNotEnoughArgs}
}
if len(args) == 1 {
tokenIndex, err := strconv.Atoi(strings.Trim(args[0], " "))
if err != nil {
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrorMappingDestinationFunctionInvalidArgument}
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrMappingDestinationInvalidArg}
}
return Wildcard, []int{tokenIndex}, -1, _EMPTY_, nil
} else {
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrorMappingDestinationFunctionTooManyArguments}
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrMappingDestinationTooManyArgs}
}
}

// partition(number of partitions, token1, token2, ...)
args = getMappingFunctionArgs(partitionMappingFunctionRegEx, token)
if args != nil {
if len(args) < 2 {
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrorMappingDestinationFunctionNotEnoughArguments}
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrMappingDestinationNotEnoughArgs}
}
if len(args) >= 2 {
mappingFunctionIntArg, err := strconv.Atoi(strings.Trim(args[0], " "))
if err != nil {
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrorMappingDestinationFunctionInvalidArgument}
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrMappingDestinationInvalidArg}
}
var numPositions = len(args[1:])
tokenIndexes := make([]int, numPositions)
for ti, t := range args[1:] {
i, err := strconv.Atoi(strings.Trim(t, " "))
if err != nil {
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrorMappingDestinationFunctionInvalidArgument}
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrMappingDestinationInvalidArg}
}
tokenIndexes[ti] = i
}
Expand Down Expand Up @@ -278,17 +296,17 @@ func indexPlaceHolders(token string) (int16, []int, int32, string, error) {
args = getMappingFunctionArgs(splitMappingFunctionRegEx, token)
if args != nil {
if len(args) < 2 {
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrorMappingDestinationFunctionNotEnoughArguments}
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrMappingDestinationNotEnoughArgs}
}
if len(args) > 2 {
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrorMappingDestinationFunctionTooManyArguments}
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrMappingDestinationTooManyArgs}
}
i, err := strconv.Atoi(strings.Trim(args[0], " "))
if err != nil {
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrorMappingDestinationFunctionInvalidArgument}
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token, ErrMappingDestinationInvalidArg}
}
if strings.Contains(args[1], " ") || strings.Contains(args[1], tsep) {
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token: token, err: ErrorMappingDestinationFunctionInvalidArgument}
return BadTransform, []int{}, -1, _EMPTY_, &mappingDestinationErr{token: token, err: ErrMappingDestinationInvalidArg}
}

return Split, []int{i}, -1, args[1], nil
Expand Down
1 change: 1 addition & 0 deletions server/subject_transform_test.go
View file
Open in desktop
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