Fix SOnarQube bug on Regex

- Regex included unlimitted repetition is now limited to 99
99 nested yang container should sufice, in my experience in 5G 20-30 levels is the max I have seen
we can always record it as a 'known limitation'
-tried to improve the redability of the Regex using constant names
- Added edge-case senarios testing related to query regex

Issue-ID: CPS-89

Signed-off-by: ToineSiebelink <toine.siebelink@est.tech>
Change-Id: Ic3442bc8ca22fe66976948f9913e033245a0e41c

diff --git a/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQuery.java b/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQuery.java
index 54a6a96..7b9bfba 100644 (file)
--- a/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQuery.java
+++ b/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQuery.java
@@ -37,14 +37,18 @@ public class CpsPathQuery {
     private Object leafValue;
     private String endsWith;
 
-    public static final Pattern QUERY_CPS_PATH_WITH_SINGLE_LEAF_PATTERN =
-        Pattern.compile("((?:\\/[^\\/]+)+?)\\[\\s*@(\\S+?)\\s*=\\s*(.*?)\\s*\\]");
+    private static final String NON_CAPTURING_GROUP_1_TO_99_YANG_CONTAINERS = "((?:\\/[^\\/]+){1,99})";
 
-    public static final Pattern QUERY_CPS_PATH_ENDS_WITH_PATTERN = Pattern.compile("\\/\\/(.+)");
+    private static final String YANG_LEAF_VALUE_EQUALS_CONDITION = "\\[\\s*@(\\S+?)\\s*=\\s*(.*?)\\s*\\]";
 
-    public static final Pattern LEAF_INTEGER_VALUE_PATTERN = Pattern.compile("[-+]?\\d+");
+    private static final Pattern QUERY_CPS_PATH_WITH_SINGLE_LEAF_PATTERN =
+        Pattern.compile(NON_CAPTURING_GROUP_1_TO_99_YANG_CONTAINERS + YANG_LEAF_VALUE_EQUALS_CONDITION);
 
-    public static final Pattern LEAF_STRING_VALUE_PATTERN = Pattern.compile("['\"](.*)['\"]");
+    private static final Pattern QUERY_CPS_PATH_ENDS_WITH_PATTERN = Pattern.compile("\\/\\/(.+)");
+
+    private static final Pattern LEAF_INTEGER_VALUE_PATTERN = Pattern.compile("[-+]?\\d+");
+
+    private static final Pattern LEAF_STRING_VALUE_PATTERN = Pattern.compile("['\"](.*)['\"]");
 
     /**
      * Returns a cps path query.

diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/query/CpsPathQuerySpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/query/CpsPathQuerySpec.groovy
index 7087613..0bc99c6 100644 (file)
--- a/cps-ri/src/test/groovy/org/onap/cps/spi/query/CpsPathQuerySpec.groovy
+++ b/cps-ri/src/test/groovy/org/onap/cps/spi/query/CpsPathQuerySpec.groovy
@@ -28,20 +28,21 @@ class CpsPathQuerySpec extends Specification {
     def objectUnderTest = new CpsPathQuery()
 
     @Unroll
-    def 'Parse cps path with valid cps path and a filter for a leaf of type : #type.'() {
+    def 'Parse cps path with valid cps path and a filter with #scenario.'() {
         when: 'the given cps path is parsed'
             def result = objectUnderTest.createFrom(cpsPath)
         then: 'the query has the right type'
             result.cpsPathQueryType == CpsPathQueryType.XPATH_LEAF_VALUE
         and: 'the right query parameters are set'
-            result.xpathPrefix == '/parent-200/child-202'
+            result.xpathPrefix == expectedXpathPrefix
             result.leafName == expectedLeafName
             result.leafValue == expectedLeafValue
         where: 'the following data is used'
-            type                        | cpsPath                                                          || expectedLeafName       | expectedLeafValue
-            'String'                    | '/parent-200/child-202[@common-leaf-name=\'common-leaf-value\']' || 'common-leaf-name'     | 'common-leaf-value'
-            'Integer'                   | '/parent-200/child-202[@common-leaf-name-int=5]'                 || 'common-leaf-name-int' | 5
-            'Integer value with spaces' | '/parent-200/child-202[@common-leaf-name-int = 5]'               || 'common-leaf-name-int' | 5
+            scenario               | cpsPath                                                  || expectedXpathPrefix | expectedLeafName       | expectedLeafValue
+            'leaf of type String'  | '/parent/child[@common-leaf-name=\'common-leaf-value\']' || '/parent/child'     |'common-leaf-name'      | 'common-leaf-value'
+            'leaf of type Integer' | '/parent/child[@common-leaf-name-int=5]'                 || '/parent/child'     |'common-leaf-name-int'  | 5
+            'spaces around ='      | '/parent/child[@common-leaf-name-int = 5]'               || '/parent/child'     |'common-leaf-name-int'  | 5
+            'key in top container' | '/parent[@common-leaf-name-int=5]'                       || '/parent'           |'common-leaf-name-int'  | 5
     }
 
     @Unroll
@@ -65,8 +66,9 @@ class CpsPathQuerySpec extends Specification {
         then: 'a CpsPathException is thrown'
             thrown(CpsPathException)
         where: 'the following data is used'
-            scenario            | cpsPath
-            'no / at the start' | 'invalid-cps-path/child'
-            'float value'       | '/parent-200/child-202[@common-leaf-name-float=5.0]'
+            scenario              | cpsPath
+            'no / at the start'   | 'invalid-cps-path/child'
+            'float value'         | '/parent-200/child-202[@common-leaf-name-float=5.0]'
+            'too many containers' | '/1/2/3/4/5/6/7/8/9/10/11/12/13/14/15/16/17/18/19/20/21/22/23/24/25/26/27/28/29/30/31/32/33/34/35/36/37/38/39/40/41/42/43/44/45/46/47/48/49/50/51/52/53/54/55/56/57/58/59/60/61/62/63/64/65/66/67/68/69/70/71/72/73/74/75/76/77/78/79/80/81/82/83/84/85/86/87/88/89/90/91/92/93/94/95/96/97/98/99/100[@a=1]'
     }
 }