Fix pattern for matching SRV service names

akka · Feb 25, 2019 · ecf433e · ecf433e
1 parent 7e24f09
commit ecf433e
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 7 deletions.
diff --git a/akka-discovery/src/main/scala/akka/discovery/ServiceDiscovery.scala b/akka-discovery/src/main/scala/akka/discovery/ServiceDiscovery.scala
@@ -206,7 +206,27 @@ case object Lookup {
 
   private val SrvQuery = """^_(.+?)\._(.+?)\.(.+?)$""".r
 
-  private val DomainName = "^((?!-)[A-Za-z0-9-]{1,63}(?<!-)\\.)+[A-Za-z]{2,6}$".r
+  /**
+   * Validates domain name:
+   * - a node name has 1 to 63 chars
+   * - valid chars for a node name are: a-z, A-Z, 0-9 and -
+   * - a node name can't start with - character
+   * - a node name can't end with - character
+   * - nodes are separated by a . character
+   *
+   * Starts with a node:
+   * Node Pattern: (?!-)[A-Za-z0-9-]{1,63}(?<!-)
+   *      (?!-) => negative look ahead, first char can't be -
+   *      [A-Za-z0-9-]{1,63} => digits and letters, from 1 to 63
+   *      (?<!-) => negative look behind, last char can't be -
+   *
+   * A node can be followed by another nodes:
+   *    Pattern: (\.(?!-)[A-Za-z0-9-]{1,63}(?<!-)))*
+   *      . => starts with a . (dot)
+   *      node pattern => (?!-)[A-Za-z0-9-]{1,63}(?<!-)
+   *      * => match zero or more times
+   */
+  private val DomainName = "^((?!-)[A-Za-z0-9-]{1,63}(?<!-))((\\.(?!-)[A-Za-z0-9-]{1,63}(?<!-)))*$".r
 
   /**
    * Create a service Lookup from a string with format:
@@ -219,7 +239,6 @@ case object Lookup {
    * The string is parsed and dismembered to build a Lookup as following:
    * Lookup(serviceName).withPortName(portName).withProtocol(protocol)
    *
-   *
    * @throws NullPointerException If the passed string is null
    * @throws IllegalArgumentException If the string doesn't not conform with the SRV format
    */

diff --git a/akka-discovery/src/test/scala/akka/discovery/LookupSpec.scala b/akka-discovery/src/test/scala/akka/discovery/LookupSpec.scala
@@ -10,12 +10,17 @@ class LookupSpec extends WordSpec with Matchers with OptionValues {
 
   // SRV strings with invalid domain names
   // should fail to build lookups
-  val srvWithInvalidDomainNames = List(
+  val srvWithInvalidServiceNames = List(
     "_portName._protocol.service_name.local",
     "_portName._protocol.servicename,local",
     "_portName._protocol.servicename.local-",
     "_portName._protocol.-servicename.local")
 
+  val srvWithValidServiceNames = List(
+    "_portName._protocol.service-name",
+    "_portName._protocol.service-name.local",
+    "_portName._protocol.service-name.svc.cluster.local")
+
   // No SRV that should result in simple A/AAAA lookups
   val noSrvLookups = List(
     "portName.protocol.serviceName.local",
@@ -28,6 +33,16 @@ class LookupSpec extends WordSpec with Matchers with OptionValues {
 
   "Lookup.parseSrv" should {
 
+    "generate a SRV Lookup from a valid SRV String" in {
+      srvWithValidServiceNames.foreach { str ⇒
+        withClue(s"parsing '$str'") {
+          val lookup = Lookup.parseSrv(str)
+          lookup.portName.value shouldBe "portName"
+          lookup.protocol.value shouldBe "protocol"
+        }
+      }
+    }
+
     "generate a SRV Lookup from a SRV String" in {
       val name = "_portName._protocol.serviceName.local"
       val lookup = Lookup.parseSrv(name)
@@ -59,7 +74,7 @@ class LookupSpec extends WordSpec with Matchers with OptionValues {
     }
 
     "throw an IllegalArgumentException for any SRV with invalid domain names" in {
-      srvWithInvalidDomainNames.foreach { str ⇒
+      srvWithInvalidServiceNames.foreach { str ⇒
         withClue(s"parsing '$str'") {
           assertThrows[IllegalArgumentException] {
             Lookup.parseSrv(str)
@@ -84,8 +99,8 @@ class LookupSpec extends WordSpec with Matchers with OptionValues {
       }
     }
 
-    "return false if domain part in SRV String is an invalid domain name" in {
-      srvWithInvalidDomainNames.foreach { str ⇒
+    "return false if domain name part in SRV String is an invalid domain name" in {
+      srvWithInvalidServiceNames.foreach { str ⇒
         withClue(s"checking '$str'") {
           Lookup.isValidSrv(str) shouldBe false
         }
@@ -101,8 +116,13 @@ class LookupSpec extends WordSpec with Matchers with OptionValues {
     }
 
     "return true for a SRV with valid domain name" in {
-      Lookup.isValidSrv("_portName._protocol.serviceName.local") shouldBe true
+      srvWithValidServiceNames.foreach { str ⇒
+        withClue(s"parsing '$str'") {
+          Lookup.isValidSrv(str) shouldBe true
+        }
+      }
     }
 
   }
+
 }