Fixes #12616: ${const.dollar} in generic method parameter leads to mi…

…ssing report
Normation · May 15, 2018 · 97a9e63 · 97a9e63
1 parent 0092c82
commit 97a9e63
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 27 deletions.
diff --git a/rudder-core/src/main/scala/com/normation/rudder/services/reports/ExecutionBatch.scala b/rudder-core/src/main/scala/com/normation/rudder/services/reports/ExecutionBatch.scala
@@ -233,8 +233,10 @@ case class ComputeCompliance(
  */
 
 object ExecutionBatch extends Loggable {
-  final val matchCFEngineVars = """.*\$(\{.+\}|\(.+\)).*""".r
-  final private val replaceCFEngineVars = """\$\{.+\}|\$\(.+\)"""
+  //these patterns must be reluctant matches to avoid strange things
+  //when two variables are presents, or something like: ${foo}xxxxxx}.
+  final val matchCFEngineVars = """.*\$(\{.+?\}|\(.+?\)).*""".r
+  final private val replaceCFEngineVars = """\$\{.+?\}|\$\(.+?\)"""
 
   /**
    * containers to store common information about "what are we
@@ -270,10 +272,12 @@ object ExecutionBatch extends Loggable {
    * Takes a string, that should contains a CFEngine var ( $(xxx) or ${xxx} )
    * replace the $(xxx) (or ${xxx}) part by .*
    * and doubles all the \
-   * Returns a string that is suitable for a being used as a regexp
+   * Returns a string that is suitable for a being used as a regexp, with anything not
+   * in ".*" quoted with \Q...\E.
+   * For example, "${foo}(bar)$(baz)foo" => "\Q\E.*\Q(bar)\E.*\Qfoo\E"
    */
-  final def replaceCFEngineVars(x : String) : String = {
-    x.replaceAll(replaceCFEngineVars, ".*").replaceAll("""\\""", """\\\\""")
+  final def replaceCFEngineVars(x : String) : Pattern = {
+    Pattern.compile("""\Q"""+ x.replaceAll(replaceCFEngineVars, """\\E.*\\Q""") + """\E""")
   }
 
   case class ContextForNoAnswer(
@@ -966,7 +970,7 @@ object ExecutionBatch extends Loggable {
 
     // build the list of unexpected ComponentValueStatusReport
     // i.e value
-    val unexpectedStatusReports = {
+    val (unexpectedStatusReports, unexpectedReports) = {
       val unexpectedReports = getUnexpectedReports(
           expectedComponent.componentsValues.toList
         , filteredReports
@@ -979,15 +983,13 @@ object ExecutionBatch extends Loggable {
 
       }
 
-      for {
-        unexpectedReport <- unexpectedReports
-      } yield {
+      (unexpectedReports.map(unexpectedReport =>
         ComponentValueStatusReport(
            unexpectedReport.keyValue
          , unexpectedReport.keyValue
          , List(MessageStatusReport(ReportType.Unexpected, unexpectedReport.message))
         )
-      }
+      ), unexpectedReports)
 
     }
 
@@ -1012,7 +1014,7 @@ object ExecutionBatch extends Loggable {
       case (kind,  (value, unexpandedValue)) =>
         value match {
           case matchCFEngineVars(_) =>
-            val pattern = Pattern.compile(replaceCFEngineVars(value))
+            val pattern = replaceCFEngineVars(value)
             kind.copy(cfeVar = (unexpandedValue, pattern) :: kind.cfeVar)
           case _ => kind.copy(simple = kind.simple + ((unexpandedValue, value +: kind.simple.getOrElse(unexpandedValue, Seq()))))
         }
@@ -1068,7 +1070,7 @@ object ExecutionBatch extends Loggable {
     }
 
     // Remove all already parsed reports so we only look in remaining reports for Cfengine variables
-    val usedReports = noneReports ++ simpleReports
+    val usedReports = noneReports ++ simpleReports ++ unexpectedReports
     val remainingReports = filteredReports.diff(usedReports)
 
     // Find what reports matche what cfengine variables
@@ -1350,15 +1352,16 @@ object ExecutionBatch extends Loggable {
     , reports  : Seq[Reports]
   ) : Seq[Reports] = {
 
-    val isExpected = (head:String, s:String) => head match {
+    val isExpected = (head:String, value:String) => head match {
       case matchCFEngineVars(_) =>
-        val matchableExpected = replaceCFEngineVars(head)
-        s.matches(matchableExpected)
-      case x => x == s
+        val pattern = replaceCFEngineVars(head)
+        pattern.matcher(value).matches()
+      case x => x == value
     }
 
     keyValues match {
-      case Nil          => reports
+      case Nil          =>
+        reports
       case head :: tail =>
         getUnexpectedReports(tail, reports.filterNot(r => isExpected(head, r.keyValue)))
     }

diff --git a/rudder-core/src/test/scala/com/normation/rudder/services/reports/ExecutionBatchTest.scala b/rudder-core/src/test/scala/com/normation/rudder/services/reports/ExecutionBatchTest.scala
@@ -176,8 +176,8 @@ class ExecutionBatchTest extends Specification {
       , List("foo", "bar")
     )
 
-    val withGood = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, reports, NoAnswer, PolicyMode.Enforce)
-    val withBad  = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, badReports, NoAnswer, PolicyMode.Enforce)
+    val withGood = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, reports, ReportType.Missing, PolicyMode.Enforce)
+    val withBad  = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, badReports, Missing, PolicyMode.Enforce)
 
     "return a component globally repaired " in {
       withGood.compliance === ComplianceLevel(success = 1, repaired = 1)
@@ -230,8 +230,8 @@ class ExecutionBatchTest extends Specification {
       , List("None", "None")
       , List("None", "None")
     )
-    val withGood = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, reports, NoAnswer, PolicyMode.Enforce)
-    val withBad  = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, badReports, NoAnswer, PolicyMode.Enforce)
+    val withGood = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, reports, ReportType.Missing, PolicyMode.Enforce)
+    val withBad  = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, badReports, ReportType.Missing, PolicyMode.Enforce)
 
     "return a component with exact reporting" in {
       withGood.compliance === ComplianceLevel(repaired = 1, success = 1)
@@ -275,8 +275,8 @@ class ExecutionBatchTest extends Specification {
       , List("${sys.bla}", "${sys.foo}")
     )
 
-    val withGood = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, reports, NoAnswer, PolicyMode.Enforce)
-    val withBad   = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, badReports, NoAnswer, PolicyMode.Enforce)
+    val withGood = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, reports, ReportType.Missing, PolicyMode.Enforce)
+    val withBad   = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, badReports, ReportType.Missing, PolicyMode.Enforce)
 
     "return a component globally repaired " in {
       withGood.compliance === ComplianceLevel(success = 1, repaired = 1)
@@ -317,8 +317,8 @@ class ExecutionBatchTest extends Specification {
       , List("${rudder.node.hostname}", "${rudder.node.hostname}", "bar")
     )
 
-    val withGood = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, reports, NoAnswer, PolicyMode.Enforce)
-    val withBad  = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, badReports, NoAnswer, PolicyMode.Enforce)
+    val withGood = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, reports, ReportType.Missing, PolicyMode.Enforce)
+    val withBad  = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, badReports, ReportType.Missing, PolicyMode.Enforce)
 
     "return a component with the correct number of success and repaired" in {
       //be carefull, here the second success is for the same unexpanded as the repaire,
@@ -384,7 +384,7 @@ class ExecutionBatchTest extends Specification {
       })
 
       val t1 = System.currentTimeMillis
-      val result = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, resultReports, NoAnswer, PolicyMode.Enforce)
+      val result = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, resultReports, ReportType.Missing, PolicyMode.Enforce)
       val t2 = System.currentTimeMillis - t1
 
       val compliance = ComplianceLevel(
@@ -510,6 +510,36 @@ class ExecutionBatchTest extends Specification {
       , patterns = Success("/var/${sys.bla}") :: Success("/var/cfengine") :: Nil
       , reports  = Success("/var/cfengine") :: Success("/var/cfengine") :: Nil
     )
+
+    // handle correctly ${boo}bar} vs ${foo}xxx} (ie matches only the variable part)
+    test("matches only the variable part of variable"
+      , patterns = Repaired("${foo}xxx}") :: Success("$(bar)yyyy)")  :: Nil
+      , reports  = Success ("ayyyy)"    ) :: Repaired("bxxx}") :: Nil
+    )
+
+    // there should be nothing special about "\" even if cfengine escape them
+    test("""nothing special with \ when a ${var} is present"""
+      , patterns = Repaired("${foo}x\\x}") :: Nil
+      , reports  = Repaired("yx\\x}") :: Nil
+    )
+
+    test("""nothing special with \"""
+      , patterns = Repaired("x\\x}") :: Nil
+      , reports  = Repaired("x\\x}") :: Nil
+    )
+
+    // we need to take care of the fact that ${const.dollar} is always replaced by cfengine
+    test("consider regex special chars as normal chars"
+      , patterns = Repaired("[^foo$]") :: Success("(bar)") :: Success("""\D\p{Lower}""") :: Nil
+      , reports  = Repaired("[^foo$]") :: Success("(bar)") :: Success("""\D\p{Lower}""") :: Nil
+    )
+
+    // we need to take care of the fact that ${const.dollar} is always replaced by cfengine
+    test("correctly understand ${const.dollar}"
+      , patterns = Repaired("""[ ${const.dollar}(echo "enabled") = 'enabled' ]""") :: Success("/var/${const.dollar}cfengine") :: Nil
+      , reports  = Repaired("""[ $(echo "enabled") = 'enabled' ]""") :: Success("/var/$cfengine") :: Nil
+    )
+
   }
 
   val fullCompliance = GlobalComplianceMode(FullCompliance, 1)
@@ -892,7 +922,7 @@ class ExecutionBatchTest extends Specification {
       , List("/var/cfengine", "bar")
     )
 
-    val withGood = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, reports, NoAnswer, PolicyMode.Enforce)
+    val withGood = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, reports, ReportType.Missing, PolicyMode.Enforce)
 
     "return a component globally success " in {
       withGood.compliance === ComplianceLevel(success = 1, notApplicable = 1)