Fixes #12390: Doubled "\" in GM call

rudder-core/src/main/scala/com/normation/cfclerk/domain/Constraint.scala

@@ -43,29 +43,53 @@ import net.liftweb.common._
 
 class ConstraintException(val msg: String) extends Exception(msg)
 
-trait VTypeWithRegex {
-  def regex:Option[RegexConstraint]
-}
 /**
  * A constraint about the type of a variable
  */
 sealed trait VTypeConstraint {
   def name: String
 
-  // Escape a String to be CFengine compliant
-  // a \ will be escaped to \\
-  // a " will be escaped to \"
-  // The parameter may be null (for some legacy reason), and it should be checked
-  def escapeString(x : String) : String = {
-    if (x == null)
-      x
-    else
-      x.replaceAll("""\\""", """\\\\""").replaceAll(""""""","""\\"""")
-  }
+  /* type of the element to use in string template.
+   * we only support "string" (almost everything), and "boolean" which
+   * are used in &IF( expressions.
+   */
+  type STTYPE
+
+  /*
+   * This method does three things:
+   * - it checks if the input value is correct reguarding the field constraint
+   * - it formats the string given its internal representation.
+   * - it uses the provided escape method to escape need chars in value.
+   *
+   * The escape function is a parameter because it can change from one agent to the next.
+   */
+  def getFormatedValidated(value: String, forField: String, escapeString: String => String) : Box[STTYPE]
+}
+
+/*
+ * When we want to have a string put in string template
+ */
+sealed trait STString extends VTypeConstraint {
+  override type STTYPE = String
+  override def getFormatedValidated(value: String, forField: String, escapeString: String => String): Box[String] = Full(escapeString(value))
+}
+
+sealed trait VTypeWithRegex extends STString {
+  def regex:Option[RegexConstraint]
+}
 
-  //check if the value is compatible with that type constrain.
-  //return a Failure on error, the checked value on success.
-  def getTypedValue(value:String, forField:String) : Box[Any] = Full(escapeString(value))
+/*
+ * When we want to have a boolean put in string template
+ */
+sealed trait STBoolean extends VTypeConstraint {
+  override type STTYPE = Boolean
+  override def getFormatedValidated(value: String, forField: String, escapeString: String => String): Box[Boolean] = {
+    try {
+      Full(value.toBoolean)
+    } catch {
+      case _:Exception => Failure(s"Wrong value ${value} for field '${forField}': expecting a boolean")
+    }
+  }
 }
 
 object VTypeConstraint {
@@ -101,10 +125,10 @@ object VTypeConstraint {
   val allTypeNames = validTypes(None,Seq()).map( _.name ).mkString(", ")
 }
 
-sealed trait StringVType extends VTypeConstraint with VTypeWithRegex {
+sealed trait StringVType extends VTypeConstraint with VTypeWithRegex with STString {
   def regex: Option[RegexConstraint]
 
-  override def getTypedValue(value:String, forField:String) : Box[Any] = regex match {
+  override def getFormatedValidated(value:String, forField:String, escapeString: String => String) : Box[String] = regex match {
     case None => Full(escapeString(value))
     case Some(regex) => regex.check(value, forField).map(escapeString(_))
   }
@@ -132,13 +156,13 @@ object MailVType extends FixedRegexVType {
 
 case class IntegerVType(regex: Option[RegexConstraint] = None) extends VTypeConstraint with VTypeWithRegex  {
   override val name = "integer"
-  override def getTypedValue(value:String, forField:String) : Box[Any] = {
-    super.getTypedValue(value, forField).flatMap( _ =>
-      try {
+  override def getFormatedValidated(value:String, forField:String, escapeString: String => String) : Box[String] = {
+    super.getFormatedValidated(value, forField, escapeString).flatMap( _ =>
+      (try {
         Full(value.toInt)
       } catch {
         case _:Exception => Failure(s"Wrong value ${value} for field '${forField}': expecting an integer.")
-      }
+      }).map( _ => value )
     )
   }
 }
@@ -152,13 +176,13 @@ case class SizetbVType(regex: Option[RegexConstraint] = None) extends SizeVType
 
 case class DateTimeVType(regex: Option[RegexConstraint] = None) extends VTypeConstraint with VTypeWithRegex {
   override val name = "datetime"
-  override def getTypedValue(value:String, forField:String) : Box[Any] = {
-    super.getTypedValue(value, forField).flatMap( _ =>
-      try
+  override def getFormatedValidated(value:String, forField:String, escapeString: String => String) : Box[String] = {
+    super.getFormatedValidated(value, forField, escapeString).flatMap( _ =>
+      (try
         Full(ISODateTimeFormat.dateTimeParser.parseDateTime(value))
       catch {
         case _:Exception => Failure(s"Wrong value ${value} for field '${forField}': expecting a datetime in ISO 8601 standard.")
-      }
+      }).map( _.toString(ISODateTimeFormat.dateTime()))
     )
   }
 }
@@ -168,8 +192,8 @@ case class TimeVType(regex: Option[RegexConstraint] = None) extends VTypeConstra
 //other types
 
 // passwords
-sealed trait AbstactPassword extends VTypeConstraint {
-  override final def getTypedValue(value:String, forField:String) : Box[Any] = {
+sealed trait AbstactPassword extends VTypeConstraint with STString {
+  override final def getFormatedValidated(value:String, forField:String, escapeString: String => String) : Box[String] = {
     HashAlgoConstraint.unserialize(value).map( _._2 ).map(escapeString(_))
   }
 }
@@ -190,24 +214,18 @@ sealed trait DerivedPasswordVType extends AbstactPassword {
 final case object AixDerivedPasswordVType   extends DerivedPasswordVType { override val tpe = HashAlgoConstraint.DerivedPasswordType.AIX   }
 final case object LinuxDerivedPasswordVType extends DerivedPasswordVType { override val tpe = HashAlgoConstraint.DerivedPasswordType.Linux }
 
-case object BooleanVType extends VTypeConstraint {
+case object BooleanVType extends VTypeConstraint with STBoolean {
   override val name = "boolean"
-  override def getTypedValue(value:String, forField:String) : Box[Any] = {
-      try {
-        Full(value.toBoolean)
-      } catch {
-        case _:Exception => Failure(s"Wrong value ${value} for field '${forField}': expecting a boolean")
-      }
-  }
 }
-case object UploadedFileVType    extends VTypeConstraint { override val name = "uploadedfile" }
-case object SharedFileVType      extends VTypeConstraint { override val name = "sharedfile" }
-case object DestinationPathVType extends VTypeConstraint { override val name = "destinationfullpath" }
-case object PermVType            extends VTypeConstraint { override val name = "perm" }
-case object RawVType             extends VTypeConstraint {
+
+case object UploadedFileVType    extends VTypeConstraint with STString { override val name = "uploadedfile" }
+case object SharedFileVType      extends VTypeConstraint with STString { override val name = "sharedfile" }
+case object DestinationPathVType extends VTypeConstraint with STString { override val name = "destinationfullpath" }
+case object PermVType            extends VTypeConstraint with STString { override val name = "perm" }
+case object RawVType             extends VTypeConstraint with STString {
   override val name = "raw"
    // no escaping for raw types
-  override def getTypedValue(value:String, forField:String) : Box[Any] = Full(value)
+  override def getFormatedValidated(value:String, forField:String, escapeString: String => String) : Box[String] = Full(value)
 }
 
 case class Constraint(
@@ -226,7 +244,7 @@ case class Constraint(
         throw new ConstraintException("'%s' field must not be empty".format(varName))
       }
     } else {
-      typeName.getTypedValue(varValue, varName) match {
+      typeName.getFormatedValidated(varValue, varName, identity) match { // here, escaping is not important
         case Full(_) => //OK
         case f:Failure => throw new ConstraintException(f.messageChain)
         //we don't want that to happen

rudder-core/src/main/scala/com/normation/cfclerk/domain/VariableAndSection.scala

@@ -73,9 +73,9 @@ sealed trait Variable extends Loggable {
    */
 
 
-  def getTypedValues(): Box[Seq[Any]] = {
+  def getValidatedValue(escape: String => String): Box[Seq[Any]] = {
     bestEffort(values) { x =>
-      castValue(x)
+      castValue(x, escape)
     }
   }
 
@@ -178,11 +178,13 @@ sealed trait Variable extends Loggable {
   def copyWithAppendedValues(seq: Seq[String]): Variable
 
 
-  protected def castValue(x: String) : Box[Any] = {
+  protected def castValue(x: String, escape: String => String) : Box[Any] = {
     //we don't want to check constraint on empty value
-    // when the variable is optionnal
+    // when the variable is optionnal.
+    // But I'm not sure if I understand what is happening with a an optionnal
+    // boolean, since we are returning a string in that case :/
     if(this.spec.constraint.mayBeEmpty && x.length < 1) Full("")
-    else spec.constraint.typeName.getTypedValue(x,spec.name)
+    else spec.constraint.typeName.getFormatedValidated(x, spec.name, escape)
   }
 }
 
@@ -360,7 +362,7 @@ object Variable {
    * Check the value we intend to put in the variable
    */
   def checkValue(variable: Variable, value: String): Boolean = {
-    variable.castValue(value).isDefined
+    variable.castValue(value, identity).isDefined // here we are not interested in the escape part
   }
 }
 

rudder-core/src/main/scala/com/normation/rudder/domain/nodes/Node.scala

@@ -311,16 +311,16 @@ object JsonSerialisation {
 
   implicit class JsonParameter(x: ParameterEntry) {
     def toJson(): JObject = (
-        ( "name"     , x.parameterName  )
-      ~ ( "value"    , x.parameterValue )
+        ( "name"     , x.parameterName )
+      ~ ( "value"    , x.escapedValue  )
     )
   }
 
   implicit class JsonParameters(parameters: Set[ParameterEntry]) {
     implicit val formats = DefaultFormats
 
     def dataJson(x: ParameterEntry) : JField = {
-      JField(x.parameterName, x.parameterValue)
+      JField(x.parameterName, x.escapedValue)
     }
 
     def toDataJson(): JObject = {

rudder-core/src/main/scala/com/normation/rudder/services/policies/write/AgentSpecificLogic.scala

@@ -55,13 +55,19 @@ import com.normation.inventory.domain.Bsd
  * writing process, mainly:
  * - specific files (like expected_reports.csv for CFEngine-based agent)
  * - specific format for "bundle" sequence.
+ * - specific escape for strings
  */
 
 //containser for agent specific file written during policy generation
 final case class AgentSpecificFile(
     path: String
 )
 
+
+trait AgentSpecificStringEscape {
+  def escape(value: String): String
+}
+
 //how do we write bundle sequence / input files system variable for the agent?
 trait AgentFormatBundleVariables {
   import BuildBundleSequence._
@@ -89,13 +95,18 @@ trait WriteAgentSpecificFiles {
   def write(cfg: AgentNodeWritableConfiguration): Box[List[AgentSpecificFile]]
 }
 
-trait AgentSpecificGeneration extends AgentSpecificGenerationHandle with AgentFormatBundleVariables with WriteAgentSpecificFiles
+trait AgentSpecificGeneration
+  extends AgentSpecificGenerationHandle
+     with AgentFormatBundleVariables
+     with WriteAgentSpecificFiles
+     with AgentSpecificStringEscape
 
-// the pipeline of processing for the specific writes
-class WriteAllAgentSpecificFiles extends WriteAgentSpecificFiles {
 
+//the extendable pipeline able to find the correct agent given (agentype, os)
+
+class AgentRegister {
   /**
-   * Ordered list of handlers, init with the default agent (cfenfine for linux)
+   * Ordered list of handlers, init with the default agent (CFEngine for linux)
    */
   private[this] var pipeline: List[AgentSpecificGeneration] =  {
     CFEngineAgentSpecificGeneration :: Nil
@@ -104,21 +115,49 @@ class WriteAllAgentSpecificFiles extends WriteAgentSpecificFiles {
   /**
    * Add the support for a new agent generation type.
    */
-  def addAgentSpecificGeneration(agent: AgentSpecificGeneration): Unit = synchronized {
+  def addAgentLogic(agent: AgentSpecificGeneration): Unit = synchronized {
     //add at the end of the pipeline new generation
     pipeline = pipeline :+ agent
   }
 
-  override def write(cfg: AgentNodeWritableConfiguration): Box[List[AgentSpecificFile]] = {
+  /**
+   * Find the first agent matching the required agentType/osDetail and apply f on it.
+   * If none is found, return a failure.
+   */
+  def findMap[T](agentType: AgentType, osDetails: OsDetails)(f: AgentSpecificGeneration => Box[T]) = {
+    pipeline.find(handler => handler.handle(agentType, osDetails)) match {
+      case None    => Failure(s"We were unable to find how to create directive sequences for Agent type ${agentType.toString()} on '${osDetails.fullName}'. " +
+                            "Perhaps you are missing the corresponding plugin. If not, please report a bug")
+      case Some(h) => f(h)
+    }
+  }
+
+  /**
+   * Execute a `traverse` on the registered agent, where `f` is used when the agentType/os is handled.
+   * Exec default on other cases.
+   *
+   */
+  def traverseMap[T](agentType: AgentType, osDetails: OsDetails)(default: () => Box[List[T]], f: AgentSpecificGeneration => Box[List[T]]): Box[List[T]] = {
     (sequence(pipeline) { handler =>
-      if(handler.handle(cfg.agentType, cfg.os)) {
-        handler.write(cfg)
+      if(handler.handle(agentType, osDetails)) {
+        f(handler)
       } else {
-        Full(Nil)
+        default()
       }
     }).map( _.flatten.toList)
   }
 
+}
+
+
+// the pipeline of processing for the specific writes
+class WriteAllAgentSpecificFiles(agentRegister: AgentRegister) extends WriteAgentSpecificFiles {
+
+
+  override def write(cfg: AgentNodeWritableConfiguration): Box[List[AgentSpecificFile]] = {
+    agentRegister.traverseMap(cfg.agentType, cfg.os)( () => Full(Nil), _.write(cfg))
+  }
+
   import BuildBundleSequence.{InputFile, TechniqueBundles, BundleSequenceVariables}
   def getBundleVariables(
       agentType   : AgentType
@@ -129,11 +168,7 @@ class WriteAllAgentSpecificFiles extends WriteAgentSpecificFiles {
     , userBundles : List[TechniqueBundles]
   ) : Box[BundleSequenceVariables] = {
     //we only choose the first matching agent for that
-    pipeline.find(handler => handler.handle(agentType, osDetails)) match {
-      case None    => Failure(s"We were unable to find how to create directive sequences for Agent type ${agentType.toString()} on '${osDetails.fullName}'. " +
-                            "Perhaps you are missing the corresponding plugin. If not, please report a bug")
-      case Some(h) => Full(h.getBundleVariables(systemInputs, sytemBundles, userInputs, userBundles))
-    }
+    agentRegister.findMap(agentType, osDetails)( a => Full(a.getBundleVariables(systemInputs, sytemBundles, userInputs, userBundles)))
   }
 }
 
@@ -142,6 +177,13 @@ class WriteAllAgentSpecificFiles extends WriteAgentSpecificFiles {
 object CFEngineAgentSpecificGeneration extends AgentSpecificGeneration {
   val GENEREATED_CSV_FILENAME = "rudder_expected_reports.csv"
 
+  /* Escape string to be CFEngine compliant
+   * a \ will be escaped to \\
+   * a " will be escaped to \"
+   */
+  override def escape(value: String): String = {
+    value.replaceAll("""\\""", """\\\\""").replaceAll(""""""" , """\\"""" )
+  }
 
   /**
    * This version only handle open source unix-like (*linux, *bsd) for

rudder-core/src/main/scala/com/normation/rudder/services/policies/write/BuildBundleSequence.scala

@@ -449,7 +449,7 @@ object CfengineBundleVariables extends AgentFormatBundleVariables {
     //the promiser value (may) comes from user input, so we need to escape
     //also, get the list of bundle for each promiser.
     //and we don't need isSystem anymore
-    val escapedSeq = bundleSeq.map(x => (ParameterEntry.escapeString(x.promiser.value, AgentType.CfeCommunity), x.bundleSequence) )
+    val escapedSeq = bundleSeq.map(x => (CFEngineAgentSpecificGeneration.escape(x.promiser.value), x.bundleSequence) )
 
     //that's the length to correctly vertically align things. Most important
     //number in all Rudder !