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 20e0def Ensure that base64 encoded '+json' bodies are accepted and
decoded. (#2646)
20e0def is described below
commit 20e0def3740a6287ad9b9f54655be427e77296ec
Author: Rob Allen <r...@akrabat.com>
AuthorDate: Thu Sep 14 03:11:48 2017 +0100
Ensure that base64 encoded '+json' bodies are accepted and decoded. (#2646)
For any base64 encoded string where the content-type is set to a valid
media type with `+json` at the end, then we need to decode it.
If the response from the web action is a JSON dictionary in the body and
the content-type header is set to a `+json` media type, then send this
is acceptable.
Do not whitelist media types in the content-type header
Instead, just ensure that they are of a valid format.
---
.../scala/whisk/core/controller/WebActions.scala | 43 +++++---
.../core/controller/test/WebActionsApiTests.scala | 113 ++++++++++++++++++++-
2 files changed, 140 insertions(+), 16 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 10e1f7c..ee598fe1 100644
--- a/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
@@ -264,8 +264,9 @@ protected[core] object WhiskWebActionsApi extends
Directives {
}
/**
- * Finds the content-type in the header list and maps it to a known media
type. If it is not
- * recognized, construct a failure with appropriate message.
+ * Finds the content-type in the header list and ensures that it is a valid
format. If it is not
+ * valid, construct a failure with appropriate message.
+ * If the content-type header is missing, then return the supplied
defaultType
*/
private def findContentTypeInHeader(headers: List[RawHeader],
transid: TransactionId,
@@ -273,28 +274,40 @@ protected[core] object WhiskWebActionsApi extends
Directives {
headers.find(_.lowercaseName == `Content-Type`.lowercaseName) match {
case Some(header) =>
MediaType.parse(header.value) match {
- case Right(mediaType: MediaType) =>
- // lookup the media type specified in the content header to see if
it is a recognized type
- MediaTypes.getForKey(mediaType.mainType ->
mediaType.subType).map(Success(_)).getOrElse {
- // this is a content-type that is not recognized, reject it
- Failure(RejectRequest(BadRequest,
Messages.httpUnknownContentType)(transid))
- }
- case _ => Failure(RejectRequest(BadRequest,
Messages.httpUnknownContentType)(transid))
+ case Right(mediaType: MediaType) => Success(mediaType)
+ case _ =>
Failure(RejectRequest(BadRequest, Messages.httpUnknownContentType)(transid))
}
case None => Success(defaultType)
}
}
+ private def isJsonFamily(mt: MediaType) = {
+ mt == `application/json` || mt.value.endsWith("+json")
+ }
+
private def interpretHttpResponseAsJson(code: StatusCode,
headers: List[RawHeader],
js: JsValue,
transid: TransactionId) = {
findContentTypeInHeader(headers, transid, `application/json`) match {
- case Success(mediaType) if (mediaType == `application/json`) =>
+ // use the default akka-http response marshaler for standard
application/json
+ case Success(mediaType) if mediaType == `application/json` =>
respondWithHeaders(removeContentTypeHeader(headers)) {
complete(code, js)
}
+ // for all other json-family content-type, explicitly marshal the
response;
+ // the order of the case statement matters; isJsonFamily returns true
for application/json
+ case Success(mediaType) if isJsonFamily(mediaType) =>
+ respondWithHeaders(removeContentTypeHeader(headers)) {
+ complete(
+ code,
+ HttpEntity(
+ ContentType(
+ MediaType.customWithFixedCharset(mediaType.mainType,
mediaType.subType, HttpCharsets.`UTF-8`)),
+ js.prettyPrint))
+ }
+
case _ =>
terminate(BadRequest, Messages.httpContentTypeError)(transid,
jsonPrettyPrinter)
}
@@ -304,11 +317,13 @@ protected[core] object WhiskWebActionsApi extends
Directives {
findContentTypeInHeader(headers, transid, `text/html`).flatMap { mediaType
=>
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, _))
+ // 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
+ case _ /* ContentType.Binary */ =>
Try(Base64.getDecoder().decode(str)).map(HttpEntity(ct, _))
}
} match {
case Success(entity) =>
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 7a87882..36d2f6c 100644
--- a/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
@@ -911,6 +911,52 @@ trait WebActionsApiTests extends ControllerTestCommon with
BeforeAndAfterEach wi
}
}
+ 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 {
implicit val tid = transid()
@@ -941,6 +987,69 @@ trait WebActionsApiTests extends ControllerTestCommon with
BeforeAndAfterEach wi
}
}
}
+ }
+
+ it should s"handle http web action without base64 encoded known '+json'
response (auth? ${creds.isDefined})" in {
+ implicit val tid = transid()
+
+ Seq(
+ (JsObject("content-type" -> "application/json-patch+json".toJson), OK),
+ (JsObject("content-type" -> "text/html".toJson), BadRequest)).foreach {
+ case (headers, expectedCode) =>
+ Seq(s"$systemId/proxy/export_c.http").foreach { path =>
+ allowedMethods.foreach { m =>
+ invocationsAllowed += 1
+ actionResult = Some(
+ JsObject(
+ "headers" -> headers,
+ webApiDirectives.statusCode -> OK.intValue.toJson,
+ "body" -> JsObject("field" -> "value".toJson)))
+
+ m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check
{
+ println(responseAs[String])
+ status should be(expectedCode)
+
+ if (expectedCode == OK) {
+ mediaType.value shouldBe "application/json-patch+json"
+ responseAs[String].parseJson shouldBe JsObject("field" ->
"value".toJson)
+ } else {
+ confirmErrorWithTid(responseAs[JsObject],
Some(Messages.httpContentTypeError))
+ }
+ }
+ }
+ }
+ }
+ }
+
+ it should s"handle http web action without base64 encoded unknown '+json'
response (auth? ${creds.isDefined})" in {
+ implicit val tid = transid()
+
+ Seq(
+ (JsObject("content-type" -> "application/hal+json".toJson), OK),
+ (JsObject("content-type" -> "text/html".toJson), BadRequest)).foreach {
+ case (headers, expectedCode) =>
+ Seq(s"$systemId/proxy/export_c.http").foreach { path =>
+ allowedMethods.foreach { m =>
+ invocationsAllowed += 1
+ actionResult = Some(
+ JsObject(
+ "headers" -> headers,
+ webApiDirectives.statusCode -> OK.intValue.toJson,
+ "body" -> JsObject("field" -> "value".toJson)))
+
+ m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check
{
+ status should be(expectedCode)
+
+ if (expectedCode == OK) {
+ mediaType.value shouldBe "application/hal+json"
+ responseAs[String].parseJson shouldBe JsObject("field" ->
"value".toJson)
+ } else {
+ confirmErrorWithTid(responseAs[JsObject],
Some(Messages.httpContentTypeError))
+ }
+ }
+ }
+ }
+ }
Seq(s"$systemId/proxy/export_c.http").foreach { path =>
allowedMethods.foreach { m =>
@@ -1018,7 +1127,7 @@ trait WebActionsApiTests extends ControllerTestCommon
with BeforeAndAfterEach wi
}
}
- it should s"reject http web action with unknown header (auth?
${creds.isDefined})" in {
+ it should s"reject http web action with invalid content-type header (auth?
${creds.isDefined})" in {
implicit val tid = transid()
Seq(s"$systemId/proxy/export_c.http").foreach { path =>
@@ -1026,7 +1135,7 @@ trait WebActionsApiTests extends ControllerTestCommon
with BeforeAndAfterEach wi
invocationsAllowed += 1
actionResult = Some(
JsObject(
- "headers" -> JsObject("content-type" -> "xyz/bar".toJson),
+ "headers" -> JsObject("content-type" -> "xyzbar".toJson),
webApiDirectives.statusCode -> OK.intValue.toJson,
"body" -> "hello world".toJson))
--
To stop receiving notification emails like this one, please contact
['"commits@openwhisk.apache.org" <commits@openwhisk.apache.org>'].
