dubee closed pull request #3919: Allow raw JSON string as response for
webaction without base64 encoding.
URL: https://github.com/apache/incubator-openwhisk/pull/3919
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/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
b/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
index e195561b04..6015525511 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 84d4104c15..a4eebbc4be 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 8933dbea01..b69fb9a6c4 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 4f945cbb34..bd1c4bef23 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 97b903a4ca..bee7a02dd2 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 7a2447cf13..2c41afa5fc 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 62bef1968c..61f374bd54 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 b500fce265..98df205766 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
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
