From 2001dec456e549c9bb0965aa2ef7cd47aaa3f9c3 Mon Sep 17 00:00:00 2001 From: Will Richard Date: Wed, 5 Mar 2014 15:28:10 -0500 Subject: [PATCH 1/6] Fixed bug reported in issue #118, where bash variables are invalid if they have "-" in their name. --- app/controllers/ApiResponse.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/ApiResponse.scala b/app/controllers/ApiResponse.scala index b9f56c00a..4dc7468f6 100644 --- a/app/controllers/ApiResponse.scala +++ b/app/controllers/ApiResponse.scala @@ -160,7 +160,7 @@ trait ApiResponse extends Controller { 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=%s;".format(prefix, k.replace('-', '_'), formatBasic(o)) //Variables with '-' in their name are invalid for bash. } }.mkString("\n") } From 0a014c1de55601bab3a3a9d98a469e48945ee0ff Mon Sep 17 00:00:00 2001 From: Will Richard Date: Wed, 5 Mar 2014 16:42:27 -0500 Subject: [PATCH 2/6] Added test that verifies that we change dashes to underscores for variable names in bash, but not other api end points. --- app/controllers/Api.scala | 2 +- test/controllers/ApiSpec.scala | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/controllers/Api.scala b/app/controllers/Api.scala index 984e9296e..d6454f43f 100644 --- 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/test/controllers/ApiSpec.scala b/test/controllers/ApiSpec.scala index 149a078f0..2ee415722 100644 --- 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" } } From 48d7b155ec04217929cd76d1cc6138e5424e0a5b Mon Sep 17 00:00:00 2001 From: Gabe Conradi Date: Thu, 6 Mar 2014 16:57:48 -0500 Subject: [PATCH 3/6] add formatPosixKey function --- app/controllers/ApiResponse.scala | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/controllers/ApiResponse.scala b/app/controllers/ApiResponse.scala index 4dc7468f6..e2f6b8362 100644 --- a/app/controllers/ApiResponse.scala +++ b/app/controllers/ApiResponse.scala @@ -154,13 +154,21 @@ 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) { + ("""[^a-zA-Z_]""".r.replaceAllIn(key.head.toString,"_") + """[^a-zA-Z0-9_]""".r.replaceAllIn(key.tail,"_")).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.replace('-', '_'), formatBasic(o)) //Variables with '-' in their name are invalid for bash. + case o => "%s%s=%s;".format(formatPosixKey(prefix + k), formatBasic(o)) } }.mkString("\n") } From 27b1d416a4211e2cf0c1ce924d15cf3a54d7c8bc Mon Sep 17 00:00:00 2001 From: Gabe Conradi Date: Fri, 7 Mar 2014 13:50:17 -0500 Subject: [PATCH 4/6] make formatPosixKey behave better --- app/controllers/ApiResponse.scala | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/controllers/ApiResponse.scala b/app/controllers/ApiResponse.scala index e2f6b8362..614b4c2c6 100644 --- a/app/controllers/ApiResponse.scala +++ b/app/controllers/ApiResponse.scala @@ -157,7 +157,12 @@ trait ApiResponse extends Controller { // formats a key to an acceptable POSIX environment variable name def formatPosixKey(key: String): String = if (!key.isEmpty) { - ("""[^a-zA-Z_]""".r.replaceAllIn(key.head.toString,"_") + """[^a-zA-Z0-9_]""".r.replaceAllIn(key.tail,"_")).toUpperCase + 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") } From b22e0dd5e61955208e6b78cd5eb13224f098ac7a Mon Sep 17 00:00:00 2001 From: Will Richard Date: Fri, 7 Mar 2014 15:08:17 -0500 Subject: [PATCH 5/6] Fix spacing. --- app/controllers/ApiResponse.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/ApiResponse.scala b/app/controllers/ApiResponse.scala index 614b4c2c6..a529507d1 100644 --- a/app/controllers/ApiResponse.scala +++ b/app/controllers/ApiResponse.scala @@ -161,7 +161,7 @@ trait ApiResponse extends Controller { val posixTailRegex = """[^a-zA-Z0-9_]""".r key.head.toString match { case posixHeadRegex() => formatPosixKey("_" + key) - case _ => posixTailRegex.replaceAllIn(key,"_").toUpperCase + case _ => posixTailRegex.replaceAllIn(key,"_").toUpperCase } } else { throw new Exception("Cannot convert an empty key into a POSIX environment variable name") From fa749bb5186006c50fa27051c9e77ee200266c9f Mon Sep 17 00:00:00 2001 From: Will Richard Date: Fri, 7 Mar 2014 15:08:35 -0500 Subject: [PATCH 6/6] Only try to format one string, since prefix and key are now combined inside of the call to formatPosixKey. --- app/controllers/ApiResponse.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/ApiResponse.scala b/app/controllers/ApiResponse.scala index a529507d1..9b56b2bda 100644 --- a/app/controllers/ApiResponse.scala +++ b/app/controllers/ApiResponse.scala @@ -173,7 +173,7 @@ trait ApiResponse extends Controller { 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(formatPosixKey(prefix + k), formatBasic(o)) + case o => "%s=%s;".format(formatPosixKey(prefix + k), formatBasic(o)) } }.mkString("\n") }