Merge pull request tumblr#129 from tumblr/will-replace-dash-in-shells…

…cript-variables Ensure variable names are valid POSIX names when using text/x-shellscript API endpoint.
Shopify · Mar 7, 2014 · b2fd49e · b2fd49e
2 parents 386973a + 6e5ace4
commit b2fd49e
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 3 deletions.
diff --git a/app/controllers/Api.scala b/app/controllers/Api.scala
@@ -49,7 +49,7 @@ IpAddressApi with AssetStateApi with AdminApi {
           "TestList" -> JsArray(List(JsNumber(1), JsNumber(2)))
         )),
         "TestList" -> JsArray(List(
-          JsObject(Seq("id" -> JsNumber(123), "name" -> JsString("foo123"))),
+          JsObject(Seq("id" -> JsNumber(123), "name" -> JsString("foo123"), "key-with-dash" -> JsString("val-with-dash"))),
           JsObject(Seq("id" -> JsNumber(124), "name" -> JsString("foo124"))),
           JsObject(Seq("id" -> JsNumber(124), "name" -> JsString("foo124")))
         ))

diff --git a/app/controllers/ApiResponse.scala b/app/controllers/ApiResponse.scala
@@ -154,13 +154,26 @@ trait ApiResponse extends Controller {
         throw new Exception("Invalid JS specified")
       }
     }
+
+    // formats a key to an acceptable POSIX environment variable name
+    def formatPosixKey(key: String): String = if (!key.isEmpty) {
+      val posixHeadRegex = """^[^a-zA-Z_]""".r
+      val posixTailRegex = """[^a-zA-Z0-9_]""".r
+      key.head.toString match {
+        case posixHeadRegex() => formatPosixKey("_" + key)
+        case _                => posixTailRegex.replaceAllIn(key,"_").toUpperCase
+      }
+    } else {
+      throw new Exception("Cannot convert an empty key into a POSIX environment variable name")
+    }
+
     // FIXME
     require(jsobject.isInstanceOf[JsObject], "Required a JsObject")
     jsobject.asInstanceOf[JsObject].value.map { case(k, v) =>
       v match {
         case m: JsObject => formatBashResponse(m, "%s_".format(prefix + k))
         case JsArray(list) => formatList(list, "%s_".format(prefix + k))
-        case o => "%s%s=%s;".format(prefix, k, formatBasic(o))
+        case o => "%s=%s;".format(formatPosixKey(prefix + k), formatBasic(o))
       }
     }.mkString("\n")
   }

diff --git a/test/controllers/ApiSpec.scala b/test/controllers/ApiSpec.scala
@@ -82,6 +82,7 @@ class ApiSpec extends ApplicationSpecification with ControllerSpec {
     override def expectedStatusCode = 200
     override def responseMatches(txt: String): Boolean = {
       txt.contains("""Data_TestList_0_name="foo123";""") &&
+      txt.contains("""Data_TestList_0_key_with_dash="val-with-dash";""") && 
       txt.contains("""Status="Ok";""");
     }
   }
@@ -96,7 +97,8 @@ class ApiSpec extends ApplicationSpecification with ControllerSpec {
       (jsData \ "Data").isInstanceOf[JsObject] &&
       (jsData \ "Data" \ "TestList").isInstanceOf[JsArray] &&
       (jsData \ "Data" \ "TestList")(0).isInstanceOf[JsObject] &&
-      ((jsData \ "Data" \ "TestList")(0) \ "id").as[Long] == 123L
+      ((jsData \ "Data" \ "TestList")(0) \ "id").as[Long] == 123L &&
+      ((jsData \ "Data" \ "TestList") (0) \ "key-with-dash").as[String] == "val-with-dash"
     }
   }