This is an automated email from the ASF dual-hosted git repository.
dubeejw 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 b6720b0 Allow raw JSON string as response for webaction without
base64 encoding. (#3919)
b6720b0 is described below
commit b6720b032a5705837622d04c590905e615e79381
Author: rodric rabbah <rod...@gmail.com>
AuthorDate: Thu Aug 9 13:10:29 2018 -0400
Allow raw JSON string as response for webaction without base64 encoding.
(#3919)
Adds test for binary entity that is base64 encoded by the controller.
Removes base64 encoding of JSON response in routemgmt package. Adjust tests
accordingly.
Removes use of base64 for webaction response in test fixture.
Co-authored-by: Carlos Santana <csantan...@apache.org>
---
.../scala/whisk/core/controller/WebActions.scala | 8 +-
core/routemgmt/common/apigw-utils.js | 26 ++---
tests/dat/actions/echo-web-http.js | 4 +-
.../{base64Web.js => jsonStringWebAction.js} | 2 +-
.../whisk/core/cli/test/ApiGwRestBasicTests.scala | 2 +-
.../scala/whisk/core/cli/test/ApiGwRestTests.scala | 4 +-
.../whisk/core/cli/test/WskWebActionsTests.scala | 6 +-
.../core/controller/test/WebActionsApiTests.scala | 126 +++++++--------------
8 files changed, 64 insertions(+), 114 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 e195561..6015525 100644
--- a/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
@@ -324,12 +324,10 @@ protected[core] object WhiskWebActionsApi extends
Directives {
findContentTypeInHeader(headers, transid, `text/html`).flatMap { mediaType
=>
val ct = ContentType(mediaType, () => HttpCharsets.`UTF-8`)
ct match {
- // base64 encoded json will appear as non-binary but it is excluded
here for legacy reasons
- case nonbinary: ContentType.NonBinary if !isJsonFamily(mediaType) =>
Success(HttpEntity(nonbinary, str))
+ case nonbinary: ContentType.NonBinary => Success(HttpEntity(nonbinary,
str))
// because of the default charset provided to the content type
constructor
- // the remaining content types to match against are binary at this
point, or
- // the legacy base64 encoded json
+ // the remaining content types to match against are binary at this
point
case _ /* ContentType.Binary */ =>
Try(Base64.getDecoder().decode(str)).map(HttpEntity(ct, _))
}
} match {
@@ -592,7 +590,7 @@ trait WhiskWebActionsApi extends Directives with
ValidateRequestSize with PostAc
}
case HttpEntity.Strict(contentType, data) =>
- // application/json is not a binary type in Akka, but is binary in
Spray
+ // for legacy, we are encoding application/json still
if (contentType.mediaType.binary || contentType.mediaType ==
`application/json`) {
Try(JsString(Base64.getEncoder.encodeToString(data.toArray)))
match {
case Success(bytes) => process(Some(bytes), isRawHttpAction)
diff --git a/core/routemgmt/common/apigw-utils.js
b/core/routemgmt/common/apigw-utils.js
index 8187633..26df324 100644
--- a/core/routemgmt/common/apigw-utils.js
+++ b/core/routemgmt/common/apigw-utils.js
@@ -529,8 +529,6 @@ function addEndpointToSwaggerApi(swaggerApi, endpoint,
responsetype) {
responsetype = responsetype || 'json';
console.log('addEndpointToSwaggerApi: operationid = '+operationId);
try {
- var auth_base64 =
Buffer.from(endpoint.action.authkey,'ascii').toString('base64');
-
// If the relative path already exists, append to it; otherwise create it
if (!swaggerApi.paths[endpoint.gatewayPath]) {
swaggerApi.paths[endpoint.gatewayPath] = {};
@@ -890,7 +888,7 @@ function replaceNamespaceInUrl(url, namespace) {
* {
* statusCode: 502, <- signifies an application error
* headers: {'Content-Type': 'application/json'},
- * body: Base64 encoded JSON error string
+ * body: JSON object or JSON string
* }
* Otherwise, the action was invoked as a regular OpenWhisk action
* (i.e. https://OW-HOST/api/v1/namesapces/NS/actions/ACTION) and the
@@ -909,18 +907,16 @@ function makeErrorResponseObject(err, isWebAction) {
return err;
}
- var bodystr;
+ var bodystr = err;
if (typeof err === 'string') {
- bodystr = JSON.stringify({
+ bodystr = {
"error": JSON.parse(makeJsonString(err)), // Make sure err is plain old
string to avoid duplicate JSON escaping
- });
- } else {
- bodystr = JSON.stringify(err);
+ };
}
return {
statusCode: 502,
headers: { 'Content-Type': 'application/json' },
- body: new Buffer(bodystr).toString('base64'),
+ body: bodystr
};
}
@@ -935,7 +931,7 @@ function makeErrorResponseObject(err, isWebAction) {
* {
* statusCode: 200, <- signifies a successful action
* headers: {'Content-Type': 'application/json'},
- * body: Base64 encoded JSON error string
+ * body: JSON object or JSON string
* }
* Otherwise, the action was invoked as a regular OpenWhisk action
* (i.e. https://OW-HOST/api/v1/namesapces/NS/actions/ACTION) and the
@@ -950,20 +946,18 @@ function makeErrorResponseObject(err, isWebAction) {
function makeResponseObject(resp, isWebAction) {
console.log('makeResponseObject: isWebAction: '+isWebAction);
if (!isWebAction) {
- console.log('makeErrorResponseObject: not called as a web action');
+ console.log('makeResponseObject: not called as a web action');
return resp;
}
- var bodystr;
+ var bodystr = resp;
if (typeof resp === 'string') {
- bodystr = makeJsonString(resp);
- } else {
- bodystr = JSON.stringify(resp);
+ bodystr = JSON.parse(makeJsonString(resp));
}
retobj = {
statusCode: 200,
headers: { 'Content-Type': 'application/json' },
- body: new Buffer(bodystr).toString('base64')
+ body: bodystr
};
return retobj;
}
diff --git a/tests/dat/actions/echo-web-http.js
b/tests/dat/actions/echo-web-http.js
index 8933dbe..b69fb9a 100644
--- a/tests/dat/actions/echo-web-http.js
+++ b/tests/dat/actions/echo-web-http.js
@@ -2,11 +2,9 @@
// license agreements; and to You under the Apache License, Version 2.0.
function main(params) {
- var bodyobj = params || {};
- bodystr = JSON.stringify(bodyobj);
return {
statusCode: 200,
headers: { 'Content-Type': 'application/json' },
- body: new Buffer(bodystr).toString('base64')
+ body: params
};
}
diff --git a/tests/dat/actions/base64Web.js
b/tests/dat/actions/jsonStringWebAction.js
similarity index 78%
rename from tests/dat/actions/base64Web.js
rename to tests/dat/actions/jsonStringWebAction.js
index 4f945cb..bd1c4be 100644
--- a/tests/dat/actions/base64Web.js
+++ b/tests/dat/actions/jsonStringWebAction.js
@@ -7,6 +7,6 @@ function main() {
"content-type": "application/json"
},
statusCode: 200,
- body: new Buffer(JSON.stringify({'status':
'success'})).toString('base64')
+ body: '{"status": "success"}'
}
}
diff --git a/tests/src/test/scala/whisk/core/cli/test/ApiGwRestBasicTests.scala
b/tests/src/test/scala/whisk/core/cli/test/ApiGwRestBasicTests.scala
index 0947225..5b31cc6 100644
--- a/tests/src/test/scala/whisk/core/cli/test/ApiGwRestBasicTests.scala
+++ b/tests/src/test/scala/whisk/core/cli/test/ApiGwRestBasicTests.scala
@@ -97,7 +97,7 @@ abstract class ApiGwRestBasicTests extends BaseApiGwTests {
}
def verifyApiGet(rr: RunResult): Unit = {
- rr.stdout should include regex
(s""""operationId":\\s+"getPathWithSub_pathsInIt"""")
+ rr.stdout should include regex
(s""""operationId":\\s*"getPathWithSub_pathsInIt"""")
}
def verifyApiFullList(rr: RunResult,
diff --git a/tests/src/test/scala/whisk/core/cli/test/ApiGwRestTests.scala
b/tests/src/test/scala/whisk/core/cli/test/ApiGwRestTests.scala
index 7a2447c..2c41afa 100644
--- a/tests/src/test/scala/whisk/core/cli/test/ApiGwRestTests.scala
+++ b/tests/src/test/scala/whisk/core/cli/test/ApiGwRestTests.scala
@@ -142,7 +142,7 @@ class ApiGwRestTests extends ApiGwRestBasicTests with
RestUtil with WskActorSyst
}
override def verifyApiGet(rr: RunResult): Unit = {
- rr.stdout should include(s""""operationId":"getPathWithSub_pathsInIt"""")
+ rr.stdout should include regex
(s""""operationId":\\s*"getPathWithSub_pathsInIt"""")
}
override def verifyApiFullList(rr: RunResult,
@@ -202,7 +202,7 @@ class ApiGwRestTests extends ApiGwRestBasicTests with
RestUtil with WskActorSyst
val openwhisk = RestResult.getFieldJsObject(urlop, "x-openwhisk")
val actionN = RestResult.getField(openwhisk, "action")
actionN shouldBe actionName
- rr.stdout should include regex
(s""""target-url":".*${actionName}.${responseType}"""")
+ rr.stdout should include regex
(s""""target-url":\\s*".*${actionName}.${responseType}"""")
}
override def verifyInvalidSwagger(rr: RunResult): Unit = {
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 62bef19..61f374b 100644
--- a/tests/src/test/scala/whisk/core/cli/test/WskWebActionsTests.scala
+++ b/tests/src/test/scala/whisk/core/cli/test/WskWebActionsTests.scala
@@ -265,9 +265,9 @@ class WskWebActionsTests extends TestHelpers with
WskTestHelpers with RestUtil w
new Header("Set-Cookie", "c=d"))
}
- it should "handle http web action with base64 encoded response" in
withAssetCleaner(wskprops) { (wp, assetHelper) =>
- val name = "base64Web"
- val file = Some(TestUtils.getTestActionFilename("base64Web.js"))
+ it should "handle http web action returning JSON as string" in
withAssetCleaner(wskprops) { (wp, assetHelper) =>
+ val name = "jsonStringWebAction"
+ val file = Some(TestUtils.getTestActionFilename("jsonStringWebAction.js"))
val host = getServiceURL
val url = host + s"$testRoutePath/$namespace/default/$name.http"
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 e74fbc2..d1ddb8b 100644
--- a/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
@@ -168,6 +168,12 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
allowedMethodsWithEntity ++ Seq(Head)
}
+ val pngSample =
"iVBORw0KGgoAAAANSUhEUgAAAAoAAAAGCAYAAAD68A/GAAAA/klEQVQYGWNgAAEHBxaG//+ZQMyyn581Pfas+cRQnf1LfF"
+
+
"Ljf+62smUgcUbt0FA2Zh7drf/ffMy9vLn3RurrW9e5hCU11i2azfD4zu1/DHz8TAy/foUxsXBrFzHzC7r8+M9S1vn1qxQT07dDjL"
+
+
"9fdemrqKxlYGT6z8AIMo6hgeUfA0PUvy9fGFh5GWK3z7vNxSWt++jX99+8SoyiGQwsW38w8PJEM7x5v5SJ8f+/xv8MDAzffv9hev"
+
+
"fkWjiXBGMpMx+j2awovjcMjFztDO8+7GF49LkbZDCDeXLTWnZO7qDfn1/+5jbw/8pjYWS4wZLztXnuEuYTk2M+MzIw/AcA36Vewa"
+
+ "D6fzsAAAAASUVORK5CYII="
+
// there is only one package that is predefined 'proxy'
val packages = Seq(
WhiskPackage(
@@ -840,7 +846,7 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
}
}
- it should s"handle http web action with base64 encoded JSON response
(auth? ${creds.isDefined})" in {
+ it should s"handle http web action with JSON object as string response
(auth? ${creds.isDefined})" in {
implicit val tid = transid()
Seq(s"$systemId/proxy/export_c.http").foreach { path =>
@@ -851,12 +857,11 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
JsObject(
"headers" -> JsObject("content-type" ->
"application/json".toJson),
webApiDirectives.statusCode -> statusCode.intValue.toJson,
- "body" -> Base64.getEncoder.encodeToString {
- JsObject("field" -> "value".toJson).compactPrint.getBytes
- }.toJson))
+ "body" -> JsObject("field" ->
"value".toJson).compactPrint.toJson))
m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
status should be(statusCode)
+ mediaType shouldBe MediaTypes.`application/json`
responseAs[JsObject] shouldBe JsObject("field" -> "value".toJson)
}
}
@@ -874,9 +879,7 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
actionResult = Some(
JsObject(
"headers" -> JsObject("content-type" ->
"application/json".toJson),
- "body" -> Base64.getEncoder.encodeToString {
- JsObject("field" -> "value".toJson).compactPrint.getBytes
- }.toJson))
+ "body" -> JsObject("field" -> "value".toJson)))
m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
status should be(OK)
@@ -887,9 +890,7 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
// omit status code and headers
allowedMethods.foreach { m =>
invocationsAllowed += 1
- actionResult = Some(JsObject("body" ->
Base64.getEncoder.encodeToString {
- JsObject("field" -> "value".toJson).compactPrint.getBytes
- }.toJson))
+ actionResult = Some(JsObject("body" -> JsObject("field" ->
"value".toJson).compactPrint.toJson))
m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
status should be(OK)
@@ -904,9 +905,7 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
actionResult = Some(
JsObject(
webApiDirectives.statusCode -> Created.intValue.toJson,
- "body" -> Base64.getEncoder.encodeToString {
- JsObject("field" -> "value".toJson).compactPrint.getBytes
- }.toJson))
+ "body" -> JsObject("field" ->
"value".toJson).compactPrint.toJson))
m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
status should be(Created)
@@ -985,53 +984,7 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
}
}
- it should s"handle http web action with base64 encoded known '+json'
response (auth? ${creds.isDefined})" in {
- implicit val tid = transid()
-
- Seq(s"$systemId/proxy/export_c.http").foreach { path =>
- allowedMethods.foreach { m =>
- invocationsAllowed += 1
- actionResult = Some(
- JsObject(
- "headers" -> JsObject("content-type" ->
"application/json-patch+json".toJson),
- webApiDirectives.statusCode -> OK.intValue.toJson,
- "body" -> Base64.getEncoder.encodeToString {
- JsObject("field" -> "value".toJson).compactPrint.getBytes
- }.toJson))
-
- m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
- status should be(OK)
- mediaType.value shouldBe "application/json-patch+json"
- responseAs[String].parseJson shouldBe JsObject("field" ->
"value".toJson)
- }
- }
- }
- }
-
- it should s"handle http web action with base64 encoded unknown '+json'
response (auth? ${creds.isDefined})" in {
- implicit val tid = transid()
-
- Seq(s"$systemId/proxy/export_c.http").foreach { path =>
- allowedMethods.foreach { m =>
- invocationsAllowed += 1
- actionResult = Some(
- JsObject(
- "headers" -> JsObject("content-type" ->
"application/hal+json".toJson),
- webApiDirectives.statusCode -> OK.intValue.toJson,
- "body" -> Base64.getEncoder.encodeToString {
- JsObject("field" -> "value".toJson).compactPrint.getBytes
- }.toJson))
-
- m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
- status should be(OK)
- mediaType.value shouldBe "application/hal+json"
- responseAs[String].parseJson shouldBe JsObject("field" ->
"value".toJson)
- }
- }
- }
- }
-
- it should s"handle http web action without base64 encoded JSON response
(auth? ${creds.isDefined})" in {
+ it should s"handle http web action with JSON object response (auth?
${creds.isDefined})" in {
implicit val tid = transid()
Seq(
@@ -1063,7 +1016,7 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
}
}
- it should s"handle http web action without base64 encoded known '+json'
response (auth? ${creds.isDefined})" in {
+ it should s"handle http web action for known '+json' response (auth?
${creds.isDefined})" in {
implicit val tid = transid()
Seq(
@@ -1094,7 +1047,7 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
}
}
- it should s"handle http web action without base64 encoded unknown '+json'
response (auth? ${creds.isDefined})" in {
+ it should s"handle http web action for unknown '+json' response (auth?
${creds.isDefined})" in {
implicit val tid = transid()
Seq(
@@ -1140,12 +1093,7 @@ trait WebActionsApiBaseTests extends
ControllerTestCommon with BeforeAndAfterEac
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))
+ val expectedEntity = HttpEntity(ContentType(MediaTypes.`image/png`),
Base64.getDecoder().decode(pngSample))
Seq(s"$systemId/proxy/export_c.http").foreach { path =>
allowedMethods.foreach { m =>
@@ -1153,7 +1101,7 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
actionResult = Some(
JsObject(
"headers" -> JsObject(`Content-Type`.lowercaseName ->
MediaTypes.`image/png`.toString.toJson),
- "body" -> png.toJson))
+ "body" -> pngSample.toJson))
m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
status should be(OK)
@@ -1180,7 +1128,7 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
}
}
- it should s"reject http web action with mismatch between header and
response (auth? ${creds.isDefined})" in {
+ it should s"allow web action with incorrect application/json header and
text response (auth? ${creds.isDefined})" in {
implicit val tid = transid()
Seq(s"$systemId/proxy/export_c.http").foreach { path =>
@@ -1193,8 +1141,10 @@ trait WebActionsApiBaseTests extends
ControllerTestCommon with BeforeAndAfterEac
"body" -> "hello world".toJson))
m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
- status should be(BadRequest)
- confirmErrorWithTid(responseAs[JsObject],
Some(Messages.httpContentTypeError))
+ status should be(OK)
+ mediaType shouldBe MediaTypes.`application/json`
+ headers shouldBe empty
+ responseAs[String] shouldBe "hello world"
}
}
}
@@ -1417,16 +1367,6 @@ trait WebActionsApiBaseTests extends
ControllerTestCommon with BeforeAndAfterEac
val str = "1,2,3"
invocationsAllowed = 3
- /*
- * Now supporting all content types with inlined "body".
- *
- Post(s"$testRoutePath/$systemId/proxy/export_c.json?a=b&c=d",
"1,2,3") ~> Route.seal(routes(creds)) ~> check {
- status should be(BadRequest)
- confirmErrorWithTid(responseAs[JsObject],
Some(Messages.contentTypeNotSupported))
- }
- *
- */
-
Post(s"$testRoutePath/$systemId/proxy/export_c.json",
HttpEntity(ContentTypes.`text/html(UTF-8)`, str)) ~> Route
.seal(routes(creds)) ~> check {
status should be(OK)
@@ -1697,7 +1637,7 @@ trait WebActionsApiBaseTests extends ControllerTestCommon
with BeforeAndAfterEac
}
}
- it should s"invoke web action ensuring JSON value body arguments are not
Base64 encoded (auth? ${creds.isDefined})" in {
+ it should s"invoke web action ensuring JSON value body arguments are
received as is (auth? ${creds.isDefined})" in {
implicit val tid = transid()
Seq("this is a string".toJson, JsArray(1.toJson, "str str".toJson,
false.toJson), true.toJson, 99.toJson)
@@ -1721,6 +1661,26 @@ trait WebActionsApiBaseTests extends
ControllerTestCommon with BeforeAndAfterEac
}
}
+ it should s"invoke web action ensuring binary body is base64 encoded
(auth? ${creds.isDefined})" in {
+ implicit val tid = transid()
+ val entity = HttpEntity(ContentType(MediaTypes.`image/png`),
Base64.getDecoder().decode(pngSample))
+
+ invocationsAllowed += 1
+ Post(s"$testRoutePath/$systemId/proxy/export_c.json", entity) ~>
Route.seal(routes(creds)) ~> check {
+ status should be(OK)
+ val response = responseAs[JsObject]
+ response shouldBe JsObject(
+ "pkg" -> s"$systemId/proxy".toJson,
+ "action" -> "export_c".toJson,
+ "content" -> metaPayload(
+ Post.method.name.toLowerCase,
+ Map(webApiDirectives.body -> pngSample.toJson).toJson.asJsObject,
+ creds,
+ pkgName = "proxy",
+ headers = List(RawHeader(`Content-Type`.lowercaseName,
MediaTypes.`image/png`.toString))))
+ }
+ }
+
it should s"allowed string based status code (auth? ${creds.isDefined})"
in {
implicit val tid = transid()
invocationsAllowed += 2
