You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by ra...@apache.org on 2017/09/01 01:35:00 UTC

[incubator-openwhisk] branch master updated: Fix handing of binary content-types in webactions (#2683)

This is an automated email from the ASF dual-hosted git repository.

rabbah pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-openwhisk.git


The following commit(s) were added to refs/heads/master by this push:
     new b15a52e  Fix handing of binary content-types in webactions (#2683)
b15a52e is described below

commit b15a52e103540dab64f2c06c8787eb92b55ae7eb
Author: James Dubee <jw...@us.ibm.com>
AuthorDate: Thu Aug 31 21:34:57 2017 -0400

    Fix handing of binary content-types in webactions (#2683)
---
 .../scala/whisk/core/controller/WebActions.scala   | 19 ++++++++-------
 tests/dat/actions/pngWeb.js                        | 13 ++++++++++
 .../whisk/core/cli/test/WskWebActionsTests.scala   | 25 +++++++++++++++++++
 .../core/controller/test/WebActionsApiTests.scala  | 28 +++++++++++++++++++++-
 4 files changed, 75 insertions(+), 10 deletions(-)

diff --git a/core/controller/src/main/scala/whisk/core/controller/WebActions.scala b/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
index a009ed2..ea704ab 100644
--- a/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
@@ -17,7 +17,6 @@
 
 package whisk.core.controller
 
-import java.nio.charset.StandardCharsets
 import java.util.Base64
 
 import scala.concurrent.Future
@@ -60,7 +59,7 @@ import whisk.http.ErrorResponse.terminate
 import whisk.http.Messages
 import whisk.utils.JsHelpers._
 
-protected[controller] sealed class WebApiDirectives (prefix: String = "__ow_") {
+protected[controller] sealed class WebApiDirectives(prefix: String = "__ow_") {
     // enforce the presence of an extension (e.g., .http) in the URI path
     val enforceExtension = false
 
@@ -285,16 +284,18 @@ protected[core] object WhiskWebActionsApi extends Directives {
 
     private def interpretHttpResponse(code: StatusCode, headers: List[RawHeader], str: String, transid: TransactionId) = {
         findContentTypeInHeader(headers, transid, `text/html`).flatMap { mediaType =>
-            // base64 encoded json response supported for legacy reasons
-            if (mediaType.binary || mediaType == `application/json`) {
-                Try(new String(Base64.getDecoder().decode(str), StandardCharsets.UTF_8)).map((mediaType, _))
-            } else {
-                Success(mediaType, str)
+            val ct = ContentType(mediaType, () => HttpCharsets.`UTF-8`)
+            ct match {
+                case _: ContentType.Binary | ContentType(`application/json`, _) =>
+                    // base64 encoded json response supported for legacy reasons
+                    Try(Base64.getDecoder().decode(str)).map(HttpEntity(ct, _))
+
+                case nonbinary: ContentType.NonBinary => Success(HttpEntity(nonbinary, str))
             }
         } match {
-            case Success((mediaType, data: String)) =>
+            case Success(entity) =>
                 respondWithHeaders(removeContentTypeHeader(headers)) {
-                    complete(code, HttpEntity(ContentType(MediaType.customWithFixedCharset(mediaType.mainType, mediaType.subType, HttpCharsets.`UTF-8`)), data))
+                    complete(code, entity)
                 }
 
             case Failure(RejectRequest(code, message)) =>
diff --git a/tests/dat/actions/pngWeb.js b/tests/dat/actions/pngWeb.js
new file mode 100644
index 0000000..3a644b0
--- /dev/null
+++ b/tests/dat/actions/pngWeb.js
@@ -0,0 +1,13 @@
+function main() {
+    var png = "iVBORw0KGgoAAAANSUhEUgAAAAoAAAAGCAYAAAD68A/GAAAA/klEQVQYGWNgAAEHBxaG//+ZQMyyn581Pfas+cRQnf1LfF" +
+        "Ljf+62smUgcUbt0FA2Zh7drf/ffMy9vLn3RurrW9e5hCU11i2azfD4zu1/DHz8TAy/foUxsXBrFzHzC7r8+M9S1vn1qxQT07" +
+        "dDjL9fdemrqKxlYGT6z8AIMo6hgeUfA0PUvy9fGFh5GWK3z7vNxSWt++jX99+8SoyiGQwsW38w8PJEM7x5v5SJ8f+/xv8MDA" +
+        "zffv9hevfkWjiXBGMpMx+j2awovjcMjFztDO8+7GF49LkbZDCDeXLTWnZO7qDfn1/+5jbw/8pjYWS4wZLztXnuEuYTk2M+Mz" +
+        "Iw/AcA36VewaD6fzsAAAAASUVORK5CYII="
+
+    return {
+        statusCode: 200,
+        headers: {'content-type': 'image/png'},
+        body: png
+    }
+}
diff --git a/tests/src/test/scala/whisk/core/cli/test/WskWebActionsTests.scala b/tests/src/test/scala/whisk/core/cli/test/WskWebActionsTests.scala
index 14daf1a..3e94e36 100644
--- a/tests/src/test/scala/whisk/core/cli/test/WskWebActionsTests.scala
+++ b/tests/src/test/scala/whisk/core/cli/test/WskWebActionsTests.scala
@@ -18,6 +18,7 @@
 package whisk.core.cli.test
 
 import java.nio.charset.StandardCharsets
+import java.util.Base64
 
 import scala.util.Failure
 import scala.util.Try
@@ -249,6 +250,30 @@ class WskWebActionsTests extends TestHelpers with WskTestHelpers with RestUtil w
             response.body.asString.parseJson.asJsObject shouldBe JsObject("status" -> "success".toJson)
     }
 
+    it should "handle http web action with base64 encoded binary response" in withAssetCleaner(wskprops) {
+        (wp, assetHelper) =>
+            val name = "binaryWeb"
+            val file = Some(TestUtils.getTestActionFilename("pngWeb.js"))
+            val host = getServiceURL
+            val url = host + s"$testRoutePath/$namespace/default/$name.http"
+            val png = "iVBORw0KGgoAAAANSUhEUgAAAAoAAAAGCAYAAAD68A/GAAAA/klEQVQYGWNgAAEHBxaG//+ZQMyyn581Pfas+cRQnf1LfF" +
+                "Ljf+62smUgcUbt0FA2Zh7drf/ffMy9vLn3RurrW9e5hCU11i2azfD4zu1/DHz8TAy/foUxsXBrFzHzC7r8+M9S1vn1qxQT07dDjL" +
+                "9fdemrqKxlYGT6z8AIMo6hgeUfA0PUvy9fGFh5GWK3z7vNxSWt++jX99+8SoyiGQwsW38w8PJEM7x5v5SJ8f+/xv8MDAzffv9hev" +
+                "fkWjiXBGMpMx+j2awovjcMjFztDO8+7GF49LkbZDCDeXLTWnZO7qDfn1/+5jbw/8pjYWS4wZLztXnuEuYTk2M+MzIw/AcA36Vewa" +
+                "D6fzsAAAAASUVORK5CYII="
+
+            assetHelper.withCleaner(wsk.action, name) {
+                (action, _) =>
+                    action.create(name, file, web = Some("true"))
+            }
+
+            val response = RestAssured.given().config(sslconfig).get(url)
+
+            response.statusCode shouldBe 200
+            response.header("Content-type") shouldBe "image/png"
+            response.body.asByteArray shouldBe Base64.getDecoder().decode(png)
+    }
+
     private val subdomainRegex = Seq.fill(WhiskProperties.getPartsInVanitySubdomain)("[a-zA-Z0-9]+").mkString("-")
 
     private val (vanitySubdomain, vanityNamespace, makeTestSubject) = {
diff --git a/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
index fac5076..f73c71d 100644
--- a/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
@@ -44,6 +44,7 @@ import akka.http.scaladsl.model.HttpMethods
 import akka.http.scaladsl.model.headers.`Content-Type`
 import akka.http.scaladsl.model.ContentTypes
 import akka.http.scaladsl.model.headers.RawHeader
+import akka.http.scaladsl.model.ContentType
 
 import spray.json._
 import spray.json.DefaultJsonProtocol._
@@ -811,6 +812,31 @@ trait WebActionsApiTests extends ControllerTestCommon with BeforeAndAfterEach wi
                 }
         }
 
+        it should s"handle http web action with base64 encoded binary response (auth? ${creds.isDefined})" in {
+            implicit val tid = transid()
+            val png = "iVBORw0KGgoAAAANSUhEUgAAAAoAAAAGCAYAAAD68A/GAAAA/klEQVQYGWNgAAEHBxaG//+ZQMyyn581Pfas+cRQnf1LfF" +
+                "Ljf+62smUgcUbt0FA2Zh7drf/ffMy9vLn3RurrW9e5hCU11i2azfD4zu1/DHz8TAy/foUxsXBrFzHzC7r8+M9S1vn1qxQT07dDjL" +
+                "9fdemrqKxlYGT6z8AIMo6hgeUfA0PUvy9fGFh5GWK3z7vNxSWt++jX99+8SoyiGQwsW38w8PJEM7x5v5SJ8f+/xv8MDAzffv9hev" +
+                "fkWjiXBGMpMx+j2awovjcMjFztDO8+7GF49LkbZDCDeXLTWnZO7qDfn1/+5jbw/8pjYWS4wZLztXnuEuYTk2M+MzIw/AcA36Vewa" +
+                "D6fzsAAAAASUVORK5CYII="
+            val expectedEntity = HttpEntity(ContentType(MediaTypes.`image/png`), Base64.getDecoder().decode(png))
+
+            Seq(s"$systemId/proxy/export_c.http").
+                foreach { path =>
+                    allowedMethods.foreach { m =>
+                        invocationsAllowed += 1
+                        actionResult = Some(JsObject(
+                            "headers" -> JsObject(`Content-Type`.lowercaseName -> MediaTypes.`image/png`.toString.toJson),
+                            "body" -> png.toJson))
+
+                        m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
+                            status should be(OK)
+                            response.entity shouldBe expectedEntity
+                        }
+                    }
+                }
+        }
+
         it should s"handle http web action with html/text response (auth? ${creds.isDefined})" in {
             implicit val tid = transid()
 
@@ -1303,7 +1329,7 @@ trait WebActionsApiTests extends ControllerTestCommon with BeforeAndAfterEach wi
 
                     Get(s"$testRoutePath/$path") ~> addHeader("Accept", "application/json") ~> Route.seal(routes(creds)) ~> check {
                         status should be(NotAcceptable)
-                        response shouldBe HttpResponse(NotAcceptable, entity = "Resource representation is only available with these types:\ntext/html")
+                        response shouldBe HttpResponse(NotAcceptable, entity = "Resource representation is only available with these types:\ntext/html; charset=UTF-8")
                     }
                 }
         }

-- 
To stop receiving notification emails like this one, please contact
['"commits@openwhisk.apache.org" <co...@openwhisk.apache.org>'].