From 08987bd173133053e969813e52e99917be70f68c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jean-No=C3=ABl=20Moyne?= <jnmoyne@gmail.com>
Date: Wed, 31 May 2023 16:28:31 -0700
Subject: [PATCH] 1: Improves error reporting for weighted mappings: As it was,
 any error in a weighted mapping would return a very unhelpfull error message.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

e.g. `nats-server: mappingtest.cfg:38:39: interface conversion: interface {} is []interface {}, not string`

This was because the line `err := &configErr{tk, fmt.Sprintf("Error adding mapping for %q to %q : %v", subj, v.(string), err)}` would panic on the `v.(string)` since in weighted mapping that interface{} is actually a map[string]interface{} (since there's can be more than one mapping in weighted mappings).

Now returns the actual error:

e.g. `nats-server: mappingtest.cfg:40:3: Error adding mapping for "bla" : invalid mapping destination: wildcard index out of range in {{wildcard(1)}}`

2: improves subject transform checking and catches if the destination is using a mapping function and there are no partial wildcards in the source.
Signed-off-by: Jean-Noël Moyne <jnmoyne@gmail.com>
---
 server/opts.go                   |  4 ++--
 server/subject_transform.go      | 18 ++++++++++++++++++
 server/subject_transform_test.go |  1 +
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/server/opts.go b/server/opts.go
index 2b69413c..765a9b56 100644
--- a/server/opts.go
+++ b/server/opts.go
@@ -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
 			}
@@ -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
 			}
diff --git a/server/subject_transform.go b/server/subject_transform.go
index 4ac93144..4c53a824 100644
--- a/server/subject_transform.go
+++ b/server/subject_transform.go
@@ -114,6 +114,12 @@ func NewSubjectTransformWithStrict(src, dest string, strict bool) (*subjectTrans
 				}
 			}
 
+			if npwcs == 0 {
+				if tranformType != NoTransform {
+					return nil, &mappingDestinationErr{token, ErrorMappingDestinationFunctionWildcardIndexOutOfRange}
+				}
+			}
+
 			if tranformType == NoTransform {
 				dtokMappingFunctionTypes = append(dtokMappingFunctionTypes, NoTransform)
 				dtokMappingFunctionTokenIndexes = append(dtokMappingFunctionTokenIndexes, []int{-1})
@@ -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}
+			}
+		}
 	}
 
 	return &subjectTransform{
diff --git a/server/subject_transform_test.go b/server/subject_transform_test.go
index 48ebcf55..bf06c45f 100644
--- a/server/subject_transform_test.go
+++ b/server/subject_transform_test.go
@@ -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()