Skip to content

Commit

Permalink
Uniformly treat zero-length bodies as missing
Browse files Browse the repository at this point in the history
  • Loading branch information
vkostyukov committed Jun 14, 2017
1 parent 3260c5f commit 35b77c1
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 30 deletions.
12 changes: 9 additions & 3 deletions core/src/main/scala/io/finch/endpoint/body.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.finch.endpoint

import com.twitter.concurrent.AsyncStream
import com.twitter.finagle.http.Fields
import com.twitter.io.Buf
import com.twitter.util.{Return, Throw}
import io.catbird.util.Rerunnable
Expand All @@ -17,9 +18,14 @@ private abstract class FullBody[A] extends Endpoint[A] {

final def apply(input: Input): Endpoint.Result[A] =
if (input.request.isChunked) EndpointResult.Skipped
else EndpointResult.Matched(input,
if (input.request.contentLength.isEmpty) missing
else present(input.request.content, input.request.charsetOrUtf8))
else {
val contentLength = input.request.headerMap.getOrNull(Fields.ContentLength)
val output =
if (contentLength == null || contentLength == "0") missing
else present(input.request.content, input.request.charsetOrUtf8)

EndpointResult.Matched(input, output)
}

final override def item: RequestItem = items.BodyItem
}
Expand Down
15 changes: 11 additions & 4 deletions core/src/test/scala/io/finch/BodySpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ class BodySpec extends FinchSpec {

def withBody(b: String): Input = Input.post("/").withBody[Text.Plain](Buf.Utf8(b))

checkAll("Body[String]", EntityEndpointLaws[String](stringBodyOption)(withBody).evaluating)
checkAll("Body[Int]", EntityEndpointLaws[Int](stringBodyOption)(withBody).evaluating)
checkAll("Body[Long]", EntityEndpointLaws[Long](stringBodyOption)(withBody).evaluating)
checkAll("Body[Boolean]", EntityEndpointLaws[Boolean](stringBodyOption)(withBody).evaluating)
Expand All @@ -22,18 +21,18 @@ class BodySpec extends FinchSpec {
checkAll("Body[UUID]", EntityEndpointLaws[UUID](stringBodyOption)(withBody).evaluating)

it should "respond with NotFound when it's required" in {
body[Foo, Text.Plain].apply(Input.get("/")).awaitValue() ===
body[Foo, Text.Plain].apply(Input.get("/")).awaitValue() shouldBe
Some(Throw(Error.NotPresent(items.BodyItem)))
}

it should "respond with None when it's optional" in {
body[Foo, Text.Plain].apply(Input.get("/")).awaitValue() === Some(Return(None))
bodyOption[Foo, Text.Plain].apply(Input.get("/")).awaitValue() shouldBe Some(Return(None))
}

it should "not match on streaming requests" in {
val req = Request()
req.setChunked(true)
body[Foo, Text.Plain].apply(Input.fromRequest(req)).awaitValueUnsafe() === None
body[Foo, Text.Plain].apply(Input.fromRequest(req)).awaitValueUnsafe() shouldBe None
}

it should "respond with a value when present and required" in {
Expand All @@ -49,4 +48,12 @@ class BodySpec extends FinchSpec {
bodyOption[Foo, Text.Plain].apply(i).awaitValueUnsafe().flatten === Some(f)
}
}

it should "treat 0-length bodies as empty" in {
val i = Input.post("/").withHeaders("Content-Length" -> "0")

bodyOption[Foo, Text.Plain].apply(i).awaitValueUnsafe().flatten shouldBe None
stringBodyOption(i).awaitValueUnsafe().flatten shouldBe None
binaryBodyOption(i).awaitValueUnsafe().flatten shouldBe None
}
}
22 changes: 0 additions & 22 deletions core/src/test/scala/io/finch/EndpointSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -215,28 +215,6 @@ class EndpointSpec extends FinchSpec {
}
}

it should "throw Error.NotParsed if as[A] method fails" in {
val cause = new Exception("can't parse this")
implicit val failingDecodeEntity: DecodeEntity[Foo] =
DecodeEntity.instance(_ => Throw(cause))

val foo = stringBody.as[Foo]
val fooOption = stringBodyOption.as[Foo]
val i = (s: String) => Input.post("/").withBody[Text.Plain](Buf.Utf8(s))

check { (s: String) =>
foo(i(s)).awaitValue() === Some(Throw(
Error.NotParsed(BodyItem, implicitly[ClassTag[Foo]], cause)
))
}

check { (s: String) =>
fooOption(i(s)).awaitValue() === Some(Throw(
Error.NotParsed(BodyItem, implicitly[ClassTag[Foo]], cause)
))
}
}

it should "rescue the exception occurred in it" in {
check { (i: Input, s: String, e: Exception) =>
Endpoint.liftFuture[Unit](Future.exception(e))
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/scala/io/finch/data/Foo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ object Foo {
Decode.text((b, cs) => Return(Foo(b.asString(cs))))

implicit val arbitraryFoo: Arbitrary[Foo] =
Arbitrary(Gen.alphaStr.map(Foo.apply))
Arbitrary(Gen.alphaStr.suchThat(_.nonEmpty).map(Foo.apply))
}

0 comments on commit 35b77c1

Please sign in to comment.