From 03e4a824c6ca184cd58551badd3cd078cbd929e6 Mon Sep 17 00:00:00 2001
From: "Francois @fanf42 Armand" <francois.armand@normation.com>
Date: Mon, 31 May 2021 12:52:09 +0200
Subject: [PATCH] Fixes #19340: Parent ticket brokes technique version tests

---
 .../cfclerk/domain/TechniqueVersion.scala     | 45 +++++++------------
 .../cfclerk/domain/TechniqueVersionTest.scala |  6 ++-
 2 files changed, 20 insertions(+), 31 deletions(-)

diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/domain/TechniqueVersion.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/domain/TechniqueVersion.scala
index 9972c1f3e03..fb1ced92931 100644
--- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/domain/TechniqueVersion.scala
+++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/domain/TechniqueVersion.scala
@@ -88,17 +88,27 @@ final class TechniqueVersion(val epoch: Int, val upsreamTechniqueVersion: Upstre
 
 object TechniqueVersion {
 
-  def apply(v: Version) = new TechniqueVersion(v.epoch.toInt, UpstreamTechniqueVersion(v.copy(epoch = 0)))
+  // A technique is much more strict than a general version: only numbers are allowed in
+  // the first parts
+  protected def apply(v: Version): Either[String, TechniqueVersion] = {
+    if((v.head :: v.parts).exists {
+      case PartType.Chars(_) => true
+      case _                 => false
+    }) Left("Technique version must be composed of digits") else Right(new TechniqueVersion(v.epoch.toInt, UpstreamTechniqueVersion(v.copy(epoch = 0))))
+  }
 
   def apply(value: String): TechniqueVersion = {
-    ParseVersion.parse(value) match {
-      case Right(v)  => TechniqueVersion(v)
+    (for {
+      v <- ParseVersion.parse(value)
+      t <- TechniqueVersion(v)
+    } yield t) match {
+      case Right(v)  => v
       case Left(err) => throw new TechniqueVersionFormatException(err)
     }
   }
 
   def parse(value: String): Either[String, TechniqueVersion] = {
-    ParseVersion.parse(value).map(TechniqueVersion(_))
+    ParseVersion.parse(value).flatMap(TechniqueVersion(_))
   }
 }
 
@@ -108,29 +118,6 @@ case class UpstreamTechniqueVersion(parsed: Version) extends Ordered[UpstreamTec
   }
 }
 
-object UpstreamTechniqueVersion {
-
-  def checkValid(value: String): Version = {
-  import scala.util.matching.Regex
-      if (value.isEmpty || !value(0).isDigit)
-        throw new TechniqueVersionFormatException("The upstream_version should start with a digit : " + value)
-
-      val validReg = new Regex("[A-Za-z0-9.+\\-:~]*")
-      validReg.findPrefixOf(value) match {
-        case Some(matchReg) if (matchReg != value) =>
-          throw new TechniqueVersionFormatException("The upstream_version contains invalid charaters.\n" +
-            "The upstream_version may contain only alphanumerics and " +
-            "the characters . + - : ~ (full stop, plus, hyphen, colon, tilde).")
-      case _ => ParseVersion.parse(value) match {
-        case Right(v)  => v
-        case Left(err) => throw new TechniqueVersionFormatException(err)
-      }
-    }
-  }
-
-}
-
-
 // alternative implementation with a parser
 
 /*
@@ -260,8 +247,8 @@ object ParseVersion {
   import fastparse._, NoWhitespace._
 
   def ascii = Charset.forName("US-ASCII").newEncoder()
-  // chars allowed in a version. Only ascii, non control, non space
-  def versionChar(c: Char) = ascii.canEncode(c) && !(c.isDigit || c.isControl || c.isSpaceChar || separatorChar(c))
+  // chars allowed in a version. Only ascii, non control, non space, non separator - including ":" used for epoch
+  def versionChar(c: Char) = ascii.canEncode(c) && !(c.isDigit || c.isControl || c.isSpaceChar || separatorChar(c) || c == ':')
   def separatorChar(c: Char) = List('~', '+', ',', '-', '.').contains(c)
 
   def num[_ :P] = P(CharIn("0-9").rep(1).!.map(_.toLong))
diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/cfclerk/domain/TechniqueVersionTest.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/cfclerk/domain/TechniqueVersionTest.scala
index 8eec0d4f1cb..45df73f436d 100644
--- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/cfclerk/domain/TechniqueVersionTest.scala
+++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/cfclerk/domain/TechniqueVersionTest.scala
@@ -143,12 +143,14 @@ class TechniqueVersionTest extends Specification {
 
   "Invalid version" should {
 
-    val msg1 = "The version format of a technique should be : [epoch:]upstream_version"
+    val msg1 = "Error when parsing 'a:18' as a version. Only ascii (non-control, non-space) chars are allowed in a version string"
+
     "throw a TechniqueVersionFormatException : %s".format(msg1) in {
       TechniqueVersion("a:18") must throwA[TechniqueVersionFormatException].like { case e => e.getMessage must contain(msg1) }
     }
 
-    val msg2 = "The upstream_version should start with a digit"
+    val msg2 = "Technique version must be composed of digits"
+
     "throw a TechniqueVersionFormatException : %s".format(msg2) in {
       TechniqueVersion("a15") must throwA[TechniqueVersionFormatException].like { case e => e.getMessage must contain(msg2) }
     }