Skip to content

Commit

Permalink
PR review suggestions.
Browse files Browse the repository at this point in the history
  • Loading branch information
Helena Edelson committed Jul 2, 2019
1 parent 262a905 commit cd59db2
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 10 deletions.
Expand Up @@ -89,10 +89,15 @@ class ActorPathSpec extends WordSpec with Matchers {
ActorPath.fromString("akka://mysys/user/boom/*")
}

"fail fast if invalid chars in host names when not using AddressFromURIString, e.g. docker host given name" in {
"detect invalid chars in host names when not using AddressFromURIString, e.g. docker host given name" in {
Address("akka", "sys", Some("valid"), Some(0)).hasInvalidHostCharacters shouldBe false
Address("akka", "sys", Some("in_valid"), Some(0)).hasInvalidHostCharacters shouldBe true
intercept[MalformedURLException](AddressFromURIString("akka://sys@in_valid:5001"))
}

"fail fast if the check is called when invalid chars are in host names" in {
Address("akka", "sys", Some("localhost"), Some(0)).checkHostCharacters()
intercept[IllegalArgumentException](Address("akka", "sys", Some("in_valid"), Some(0)).checkHostCharacters())
}
}
}
8 changes: 7 additions & 1 deletion akka-actor/src/main/scala/akka/actor/Address.scala
Expand Up @@ -69,12 +69,18 @@ final case class Address private (protocol: String, system: String, host: Option
*/
def hostPort: String = toString.substring(protocol.length + 3)

/** INTERNAL API
/** INTERNAL API
* Check if the address is not created through `AddressFromURIString`, if there
* are any unusual characters in the host string.
*/
@InternalApi
private[akka] def hasInvalidHostCharacters: Boolean = host.exists(_.contains("_"))

/** INTERNAL API */
@InternalApi
private[akka] def checkHostCharacters(): Unit =
require(!hasInvalidHostCharacters, s"Using invalid host characters '$host' in the Address is not allowed.")

}

object Address {
Expand Down
Expand Up @@ -81,7 +81,9 @@ sealed trait ClusterCommand
* The name of the [[akka.actor.ActorSystem]] must be the same for all members of a
* cluster.
*/
final case class Join(address: Address) extends ClusterCommand
final case class Join(address: Address) extends ClusterCommand {
address.checkHostCharacters()
}

object Join {

Expand All @@ -100,6 +102,7 @@ object Join {
* cluster or to join the same cluster again.
*/
final case class JoinSeedNodes(seedNodes: immutable.Seq[Address]) extends ClusterCommand {
seedNodes.foreach(_.checkHostCharacters())

/**
* Java API: Join the specified seed nodes without defining them in config.
Expand Down
Expand Up @@ -4,6 +4,7 @@

package akka.cluster.typed

import akka.actor.Address
import akka.actor.typed.scaladsl.adapter._
import akka.cluster.ClusterEvent._
import akka.cluster.MemberStatus
Expand Down Expand Up @@ -43,6 +44,12 @@ class ClusterApiSpec extends ScalaTestWithActorTestKit(ClusterApiSpec.config) wi

"A typed Cluster" must {

"fail fast in a join attempt if invalid chars are in host names, e.g. docker host given name" in {
val address = Address("akka", "sys", Some("in_valid"), Some(0))
intercept[IllegalArgumentException](Join(address))
intercept[IllegalArgumentException](JoinSeedNodes(scala.collection.immutable.Seq(address)))
}

"join a cluster and observe events from both sides" in {

val system2 = akka.actor.ActorSystem(system.name, system.settings.config)
Expand Down
9 changes: 2 additions & 7 deletions akka-cluster/src/main/scala/akka/cluster/Cluster.scala
Expand Up @@ -301,7 +301,7 @@ class Cluster(val system: ExtendedActorSystem) extends Extension {
* cluster.
*/
def join(address: Address): Unit = {
checkHostCharacters(address)
address.checkHostCharacters()
clusterCore ! ClusterUserAction.JoinTo(fillLocal(address))
}

Expand All @@ -320,7 +320,7 @@ class Cluster(val system: ExtendedActorSystem) extends Extension {
* cluster or to join the same cluster again.
*/
def joinSeedNodes(seedNodes: immutable.Seq[Address]): Unit = {
seedNodes.foreach(checkHostCharacters)
seedNodes.foreach(_.checkHostCharacters())
clusterCore ! InternalClusterAction.JoinSeedNodes(seedNodes.toVector.map(fillLocal))
}

Expand All @@ -337,11 +337,6 @@ class Cluster(val system: ExtendedActorSystem) extends Extension {
def joinSeedNodes(seedNodes: java.util.List[Address]): Unit =
joinSeedNodes(Util.immutableSeq(seedNodes))

private def checkHostCharacters(address: Address): Unit =
require(
!address.hasInvalidHostCharacters,
s"Joining the cluster using invalid host characters '${address.host}' in the Address is not allowed.")

/**
* Send command to issue state transition to LEAVING for the node specified by 'address'.
* The member will go through the status changes [[MemberStatus]] `Leaving` (not published to
Expand Down

0 comments on commit cd59db2

Please sign in to comment.