You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by GitBox <gi...@apache.org> on 2018/02/28 19:45:18 UTC

[GitHub] csantanapr closed pull request #3357: Remove usages of replace and replaceAll on the hotpath.

csantanapr closed pull request #3357: Remove usages of replace and replaceAll on the hotpath.
URL: https://github.com/apache/incubator-openwhisk/pull/3357
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/common/scala/src/main/scala/whisk/core/containerpool/ContainerFactory.scala b/common/scala/src/main/scala/whisk/core/containerpool/ContainerFactory.scala
index 25be520070..fd19495b1b 100644
--- a/common/scala/src/main/scala/whisk/core/containerpool/ContainerFactory.scala
+++ b/common/scala/src/main/scala/whisk/core/containerpool/ContainerFactory.scala
@@ -52,11 +52,12 @@ trait ContainerFactory {
 
 object ContainerFactory {
 
+  /** based on https://github.com/moby/moby/issues/3138 and https://github.com/moby/moby/blob/master/daemon/names/names.go */
+  private def isAllowed(c: Char) = c.isLetterOrDigit || c == '_' || c == '.' || c == '-'
+
   /** include the instance name, if specified and strip invalid chars before attempting to use them in the container name */
   def containerNamePrefix(instanceId: InstanceId): String =
-    s"wsk${instanceId.name.getOrElse("")}${instanceId.toInt}"
-      .replaceAll("[^a-zA-Z0-9_\\.\\-]", "") // based on https://github.com/moby/moby/issues/3138 and https://github.com/moby/moby/blob/master/daemon/names/names.go
-
+    s"wsk${instanceId.name.getOrElse("")}${instanceId.toInt}".filter(isAllowed)
 }
 
 /**
diff --git a/core/invoker/src/main/scala/whisk/core/containerpool/ContainerProxy.scala b/core/invoker/src/main/scala/whisk/core/containerpool/ContainerProxy.scala
index 7f37aaf231..d50e07c2af 100644
--- a/core/invoker/src/main/scala/whisk/core/containerpool/ContainerProxy.scala
+++ b/core/invoker/src/main/scala/whisk/core/containerpool/ContainerProxy.scala
@@ -427,9 +427,14 @@ object ContainerProxy {
    * @param suffix the container name's suffix
    * @return a unique container name
    */
-  def containerName(instance: InstanceId, prefix: String, suffix: String) =
-    s"${ContainerFactory.containerNamePrefix(instance)}_${containerCount.next()}_${prefix}_${suffix}"
-      .replaceAll("[^a-zA-Z0-9_]", "")
+  def containerName(instance: InstanceId, prefix: String, suffix: String): String = {
+    def isAllowed(c: Char): Boolean = c.isLetterOrDigit || c == '_'
+
+    val sanitizedPrefix = prefix.filter(isAllowed)
+    val sanitizedSuffix = suffix.filter(isAllowed)
+
+    s"${ContainerFactory.containerNamePrefix(instance)}_${containerCount.next()}_${sanitizedPrefix}_${sanitizedSuffix}"
+  }
 
   /**
    * Creates a WhiskActivation ready to be sent via active ack.


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services