Skip to content

Commit

Permalink
Merge pull request akka#17020 from spray/wip-close-tests
Browse files Browse the repository at this point in the history 
=htc Enable rendering of HTTP/1.0 responses, increase test coverage for connection closing logic
  • Loading branch information
@bantonsson
bantonsson committed Mar 14, 2015
2 parents 119ff32 + 6cf6473 commit 1b52ae3
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 46 deletions.
4 changes: 2 additions & 2 deletions akka-http-core/src/main/scala/akka/http/engine/parsing/ParserOutput.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ private[http] object ParserOutput {
headers: List[HttpHeader],
createEntity: Source[RequestOutput, Unit] ⇒ RequestEntity,
expect100ContinueResponsePending: Boolean,
closeAfterResponseCompletion: Boolean) extends MessageStart with RequestOutput
closeRequested: Boolean) extends MessageStart with RequestOutput

final case class ResponseStart(
statusCode: StatusCode,
protocol: HttpProtocol,
headers: List[HttpHeader],
createEntity: Source[ResponseOutput, Unit] ⇒ ResponseEntity,
closeAfterResponseCompletion: Boolean) extends MessageStart with ResponseOutput
closeRequested: Boolean) extends MessageStart with ResponseOutput

case object MessageEnd extends MessageOutput

Expand Down
36 changes: 26 additions & 10 deletions akka-http-core/src/main/scala/akka/http/engine/rendering/HttpResponseRendererFactory.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ private[http] class HttpResponseRendererFactory(serverHeader: Option[headers.Ser
val noEntity = entity.isKnownEmpty || ctx.requestMethod == HttpMethods.HEAD

def renderStatusLine(): Unit =
if (status eq StatusCodes.OK) r ~~ DefaultStatusLineBytes else r ~~ StatusLineStartBytes ~~ status ~~ CrLf
protocol match {
case `HTTP/1.1` ⇒ if (status eq StatusCodes.OK) r ~~ DefaultStatusLineBytes else r ~~ StatusLineStartBytes ~~ status ~~ CrLf
case `HTTP/1.0` ⇒ r ~~ protocol ~~ ' ' ~~ status ~~ CrLf
}

def render(h: HttpHeader) = r ~~ h ~~ CrLf

Expand Down Expand Up @@ -125,14 +128,27 @@ private[http] class HttpResponseRendererFactory(serverHeader: Option[headers.Ser
case Nil ⇒
if (!serverSeen) renderDefaultServerHeader(r)
if (!dateSeen) r ~~ dateHeader
close = alwaysClose ||
ctx.closeAfterResponseCompletion || // request wants to close
(connHeader != null && connHeader.hasClose) // application wants to close
ctx.requestProtocol match {
case `HTTP/1.0` if !close ⇒ r ~~ Connection ~~ KeepAliveBytes ~~ CrLf
case `HTTP/1.1` if close ⇒ r ~~ Connection ~~ CloseBytes ~~ CrLf
case _ ⇒ // no need for rendering
}

// Do we close the connection after this response?
close =
// if we are prohibited to keep-alive by the spec
alwaysClose ||
// if the client wants to close and we don't override
(ctx.closeRequested && ((connHeader eq null) || !connHeader.hasKeepAlive)) ||
// if the application wants to close explicitly
(protocol match {
case `HTTP/1.1` ⇒ (connHeader ne null) && connHeader.hasClose
case `HTTP/1.0` ⇒ if (connHeader eq null) ctx.requestProtocol == `HTTP/1.1` else !connHeader.hasKeepAlive
})

// Do we render an explicit Connection header?
val renderConnectionHeader =
protocol == `HTTP/1.0` && !close || protocol == `HTTP/1.1` && close || // if we don't follow the default behavior
close != ctx.closeRequested || // if we override the client's closing request
protocol != ctx.requestProtocol // if we reply with a mismatching protocol (let's be very explicit in this case)

if (renderConnectionHeader)
r ~~ Connection ~~ (if (close) CloseBytes else KeepAliveBytes) ~~ CrLf
if (mustRenderTransferEncodingChunkedHeader && !transferEncodingSeen)
r ~~ `Transfer-Encoding` ~~ ChunkedBytes ~~ CrLf
}
Expand Down Expand Up @@ -190,4 +206,4 @@ private[http] final case class ResponseRenderingContext(
response: HttpResponse,
requestMethod: HttpMethod = HttpMethods.GET,
requestProtocol: HttpProtocol = HttpProtocols.`HTTP/1.1`,
closeAfterResponseCompletion: Boolean = false)
closeRequested: Boolean = false)
8 changes: 4 additions & 4 deletions akka-http-core/src/main/scala/akka/http/engine/server/HttpServer.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ private[http] object HttpServer {
State[Any](ReadAny(oneHundredContinueInput.asInstanceOf[Inlet[Any]] :: applicationInput.asInstanceOf[Inlet[Any]] :: Nil)) {
case (ctx, _, response: HttpResponse) ⇒
// see the comment on [[OneHundredContinue]] for an explanation of the closing logic here (and more)
val close = requestStart.closeAfterResponseCompletion || requestStart.expect100ContinueResponsePending
val close = requestStart.closeRequested || requestStart.expect100ContinueResponsePending
ctx.emit(ResponseRenderingContext(response, requestStart.method, requestStart.protocol, close))
if (close) finish(ctx) else {
ctx.changeCompletionHandling(eagerClose)
Expand All @@ -175,7 +175,7 @@ private[http] object HttpServer {

val waitingForApplicationResponseCompletionHandling = CompletionHandling(
onUpstreamFinish = {
case (ctx, `bypassInput`) ⇒ { requestStart = requestStart.copy(closeAfterResponseCompletion = true); SameState }
case (ctx, `bypassInput`) ⇒ { requestStart = requestStart.copy(closeRequested = true); SameState }
case (ctx, _) ⇒ { ctx.finish(); SameState }
},
onUpstreamFailure = {
Expand All @@ -192,7 +192,7 @@ private[http] object HttpServer {
ctx match {
case fullCtx: MergeLogicContext ⇒
// note that this will throw IllegalArgumentException if no demand available
fullCtx.emit(ResponseRenderingContext(HttpResponse(status, entity = msg), closeAfterResponseCompletion = true))
fullCtx.emit(ResponseRenderingContext(HttpResponse(status, entity = msg), closeRequested = true))
case other ⇒ throw new IllegalStateException(s"Unexpected MergeLogicContext [${other.getClass.getName}]")
}
//
Expand Down Expand Up @@ -259,7 +259,7 @@ private[http] object HttpServer {
case NonFatal(e) ⇒
log.error(e, "Internal server error, sending 500 response")
errorResponse = ResponseRenderingContext(HttpResponse(StatusCodes.InternalServerError),
closeAfterResponseCompletion = true)
closeRequested = true)
ctx.absorbTermination()
case _ ⇒ ctx.fail(error)
}
Expand Down
149 changes: 119 additions & 30 deletions akka-http-core/src/test/scala/akka/http/engine/rendering/ResponseRendererSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ class ResponseRendererSpec extends FreeSpec with Matchers with BeforeAndAfterAll
"""HTTP/1.1 200 OK
|Server: akka-http/1.0.0
|Date: Thu, 25 Aug 2011 09:10:29 GMT
|Connection: close
|Content-Type: application/json; charset=UTF-8
|
|abcdefg""", close = true)
Expand All @@ -296,6 +297,7 @@ class ResponseRendererSpec extends FreeSpec with Matchers with BeforeAndAfterAll
"""HTTP/1.1 200 OK
|Server: akka-http/1.0.0
|Date: Thu, 25 Aug 2011 09:10:29 GMT
|Connection: close
|Content-Type: text/plain; charset=UTF-8
|
|body123""", close = true)
Expand Down Expand Up @@ -360,42 +362,129 @@ class ResponseRendererSpec extends FreeSpec with Matchers with BeforeAndAfterAll
import org.scalatest.prop.TableDrivenPropertyChecks._
import HttpProtocols._

def NONE: Option[String] = None
def NONE: Option[Connection] = None
def CLOSE: Option[Connection] = Some(Connection("close"))
def KEEPA: Option[Connection] = Some(Connection("Keep-Alive"))
// format: OFF
val table = Table(
("Client Version", "Request" , "Response" , "Rendered" , "Close"),
(`HTTP/1.1` , NONE , NONE , NONE , false),
(`HTTP/1.1` , Some("close") , NONE , Some("close") , true),
(`HTTP/1.1` , Some("Keep-Alive") , NONE , NONE , false),
(`HTTP/1.0` , NONE , NONE , NONE , true),
(`HTTP/1.0` , Some("close") , NONE , NONE , true),
(`HTTP/1.0` , Some("Keep-Alive") , NONE , Some("Keep-Alive"), false),
(`HTTP/1.1` , NONE , Some("close") , Some("close") , true))
//--- requested by the client ----// //----- set by server app -----// //--- actually done ---//
// Request Request Response Response Rendered Connection
// Request Method Connection Response Connection Entity Connection Closed after
// Protocol is HEAD Header Protocol Header CloseDelim Header Response
("Req Prot", "HEAD Req", "Req CH", "Res Prot", "Res CH", "Res CD", "Ren Conn", "Close"),
(`HTTP/1.1`, false, NONE, `HTTP/1.1`, NONE, false, NONE, false),
(`HTTP/1.1`, false, NONE, `HTTP/1.1`, NONE, true, CLOSE, true),
(`HTTP/1.1`, false, NONE, `HTTP/1.1`, CLOSE, false, CLOSE, true),
(`HTTP/1.1`, false, NONE, `HTTP/1.1`, CLOSE, true, CLOSE, true),
(`HTTP/1.1`, false, NONE, `HTTP/1.1`, KEEPA, false, NONE, false),
(`HTTP/1.1`, false, NONE, `HTTP/1.1`, KEEPA, true, CLOSE, true),
(`HTTP/1.1`, false, NONE, `HTTP/1.0`, NONE, false, CLOSE, true),
(`HTTP/1.1`, false, NONE, `HTTP/1.0`, NONE, true, CLOSE, true),
(`HTTP/1.1`, false, NONE, `HTTP/1.0`, CLOSE, false, CLOSE, true),
(`HTTP/1.1`, false, NONE, `HTTP/1.0`, CLOSE, true, CLOSE, true),
(`HTTP/1.1`, false, NONE, `HTTP/1.0`, KEEPA, false, KEEPA, false),
(`HTTP/1.1`, false, NONE, `HTTP/1.0`, KEEPA, true, CLOSE, true),
(`HTTP/1.1`, false, CLOSE, `HTTP/1.1`, NONE, false, CLOSE, true),
(`HTTP/1.1`, false, CLOSE, `HTTP/1.1`, NONE, true, CLOSE, true),
(`HTTP/1.1`, false, CLOSE, `HTTP/1.1`, CLOSE, false, CLOSE, true),
(`HTTP/1.1`, false, CLOSE, `HTTP/1.1`, CLOSE, true, CLOSE, true),
(`HTTP/1.1`, false, CLOSE, `HTTP/1.1`, KEEPA, false, KEEPA, false),
(`HTTP/1.1`, false, CLOSE, `HTTP/1.1`, KEEPA, true, CLOSE, true),
(`HTTP/1.1`, false, CLOSE, `HTTP/1.0`, NONE, false, CLOSE, true),
(`HTTP/1.1`, false, CLOSE, `HTTP/1.0`, NONE, true, CLOSE, true),
(`HTTP/1.1`, false, CLOSE, `HTTP/1.0`, CLOSE, false, CLOSE, true),
(`HTTP/1.1`, false, CLOSE, `HTTP/1.0`, CLOSE, true, CLOSE, true),
(`HTTP/1.1`, false, CLOSE, `HTTP/1.0`, KEEPA, false, KEEPA, false),
(`HTTP/1.1`, false, CLOSE, `HTTP/1.0`, KEEPA, true, CLOSE, true),
(`HTTP/1.1`, true, NONE, `HTTP/1.1`, NONE, false, NONE, false),
(`HTTP/1.1`, true, NONE, `HTTP/1.1`, NONE, true, NONE, false),
(`HTTP/1.1`, true, NONE, `HTTP/1.1`, CLOSE, false, CLOSE, true),
(`HTTP/1.1`, true, NONE, `HTTP/1.1`, CLOSE, true, CLOSE, true),
(`HTTP/1.1`, true, NONE, `HTTP/1.1`, KEEPA, false, NONE, false),
(`HTTP/1.1`, true, NONE, `HTTP/1.1`, KEEPA, true, NONE, false),
(`HTTP/1.1`, true, NONE, `HTTP/1.0`, NONE, false, CLOSE, true),
(`HTTP/1.1`, true, NONE, `HTTP/1.0`, NONE, true, CLOSE, true),
(`HTTP/1.1`, true, NONE, `HTTP/1.0`, CLOSE, false, CLOSE, true),
(`HTTP/1.1`, true, NONE, `HTTP/1.0`, CLOSE, true, CLOSE, true),
(`HTTP/1.1`, true, NONE, `HTTP/1.0`, KEEPA, false, KEEPA, false),
(`HTTP/1.1`, true, NONE, `HTTP/1.0`, KEEPA, true, KEEPA, false),
(`HTTP/1.1`, true, CLOSE, `HTTP/1.1`, NONE, false, CLOSE, true),
(`HTTP/1.1`, true, CLOSE, `HTTP/1.1`, NONE, true, CLOSE, true),
(`HTTP/1.1`, true, CLOSE, `HTTP/1.1`, CLOSE, false, CLOSE, true),
(`HTTP/1.1`, true, CLOSE, `HTTP/1.1`, CLOSE, true, CLOSE, true),
(`HTTP/1.1`, true, CLOSE, `HTTP/1.1`, KEEPA, false, KEEPA, false),
(`HTTP/1.1`, true, CLOSE, `HTTP/1.1`, KEEPA, true, KEEPA, false),
(`HTTP/1.1`, true, CLOSE, `HTTP/1.0`, NONE, false, CLOSE, true),
(`HTTP/1.1`, true, CLOSE, `HTTP/1.0`, NONE, true, CLOSE, true),
(`HTTP/1.1`, true, CLOSE, `HTTP/1.0`, CLOSE, false, CLOSE, true),
(`HTTP/1.1`, true, CLOSE, `HTTP/1.0`, CLOSE, true, CLOSE, true),
(`HTTP/1.1`, true, CLOSE, `HTTP/1.0`, KEEPA, false, KEEPA, false),
(`HTTP/1.1`, true, CLOSE, `HTTP/1.0`, KEEPA, true, KEEPA, false),
(`HTTP/1.0`, false, NONE, `HTTP/1.1`, NONE, false, CLOSE, true),
(`HTTP/1.0`, false, NONE, `HTTP/1.1`, NONE, true, CLOSE, true),
(`HTTP/1.0`, false, NONE, `HTTP/1.1`, CLOSE, false, CLOSE, true),
(`HTTP/1.0`, false, NONE, `HTTP/1.1`, CLOSE, true, CLOSE, true),
(`HTTP/1.0`, false, NONE, `HTTP/1.1`, KEEPA, false, KEEPA, false),
(`HTTP/1.0`, false, NONE, `HTTP/1.1`, KEEPA, true, CLOSE, true),
(`HTTP/1.0`, false, NONE, `HTTP/1.0`, NONE, false, NONE, true),
(`HTTP/1.0`, false, NONE, `HTTP/1.0`, NONE, true, NONE, true),
(`HTTP/1.0`, false, NONE, `HTTP/1.0`, CLOSE, false, NONE, true),
(`HTTP/1.0`, false, NONE, `HTTP/1.0`, CLOSE, true, NONE, true),
(`HTTP/1.0`, false, NONE, `HTTP/1.0`, KEEPA, false, KEEPA, false),
(`HTTP/1.0`, false, NONE, `HTTP/1.0`, KEEPA, true, NONE, true),
(`HTTP/1.0`, false, KEEPA, `HTTP/1.1`, NONE, false, KEEPA, false),
(`HTTP/1.0`, false, KEEPA, `HTTP/1.1`, NONE, true, CLOSE, true),
(`HTTP/1.0`, false, KEEPA, `HTTP/1.1`, CLOSE, false, CLOSE, true),
(`HTTP/1.0`, false, KEEPA, `HTTP/1.1`, CLOSE, true, CLOSE, true),
(`HTTP/1.0`, false, KEEPA, `HTTP/1.1`, KEEPA, false, KEEPA, false),
(`HTTP/1.0`, false, KEEPA, `HTTP/1.1`, KEEPA, true, CLOSE, true),
(`HTTP/1.0`, false, KEEPA, `HTTP/1.0`, NONE, false, KEEPA, false),
(`HTTP/1.0`, false, KEEPA, `HTTP/1.0`, NONE, true, CLOSE, true),
(`HTTP/1.0`, false, KEEPA, `HTTP/1.0`, CLOSE, false, CLOSE, true),
(`HTTP/1.0`, false, KEEPA, `HTTP/1.0`, CLOSE, true, CLOSE, true),
(`HTTP/1.0`, false, KEEPA, `HTTP/1.0`, KEEPA, false, KEEPA, false),
(`HTTP/1.0`, false, KEEPA, `HTTP/1.0`, KEEPA, true, CLOSE, true),
(`HTTP/1.0`, true, NONE, `HTTP/1.1`, NONE, false, CLOSE, true),
(`HTTP/1.0`, true, NONE, `HTTP/1.1`, NONE, true, CLOSE, true),
(`HTTP/1.0`, true, NONE, `HTTP/1.1`, CLOSE, false, CLOSE, true),
(`HTTP/1.0`, true, NONE, `HTTP/1.1`, CLOSE, true, CLOSE, true),
(`HTTP/1.0`, true, NONE, `HTTP/1.1`, KEEPA, false, KEEPA, false),
(`HTTP/1.0`, true, NONE, `HTTP/1.1`, KEEPA, true, KEEPA, false),
(`HTTP/1.0`, true, NONE, `HTTP/1.0`, NONE, false, NONE, true),
(`HTTP/1.0`, true, NONE, `HTTP/1.0`, NONE, true, NONE, true),
(`HTTP/1.0`, true, NONE, `HTTP/1.0`, CLOSE, false, NONE, true),
(`HTTP/1.0`, true, NONE, `HTTP/1.0`, CLOSE, true, NONE, true),
(`HTTP/1.0`, true, NONE, `HTTP/1.0`, KEEPA, false, KEEPA, false),
(`HTTP/1.0`, true, NONE, `HTTP/1.0`, KEEPA, true, KEEPA, false),
(`HTTP/1.0`, true, KEEPA, `HTTP/1.1`, NONE, false, KEEPA, false),
(`HTTP/1.0`, true, KEEPA, `HTTP/1.1`, NONE, true, KEEPA, false),
(`HTTP/1.0`, true, KEEPA, `HTTP/1.1`, CLOSE, false, CLOSE, true),
(`HTTP/1.0`, true, KEEPA, `HTTP/1.1`, CLOSE, true, CLOSE, true),
(`HTTP/1.0`, true, KEEPA, `HTTP/1.1`, KEEPA, false, KEEPA, false),
(`HTTP/1.0`, true, KEEPA, `HTTP/1.1`, KEEPA, true, KEEPA, false),
(`HTTP/1.0`, true, KEEPA, `HTTP/1.0`, NONE, false, KEEPA, false),
(`HTTP/1.0`, true, KEEPA, `HTTP/1.0`, NONE, true, KEEPA, false),
(`HTTP/1.0`, true, KEEPA, `HTTP/1.0`, CLOSE, false, CLOSE, true),
(`HTTP/1.0`, true, KEEPA, `HTTP/1.0`, CLOSE, true, CLOSE, true),
(`HTTP/1.0`, true, KEEPA, `HTTP/1.0`, KEEPA, false, KEEPA, false),
(`HTTP/1.0`, true, KEEPA, `HTTP/1.0`, KEEPA, true, KEEPA, false))
// format: ON

forAll(table) { (reqProto, reqCH, resCH, renCH, close) ⇒
forAll(table)((reqProto, headReq, reqCH, resProto, resCH, resCD, renCH, close) ⇒
ResponseRenderingContext(
response = HttpResponse(200, headers = resCH.map(h ⇒ List(Connection(h))) getOrElse Nil),
response = HttpResponse(200, headers = resCH.toList,
entity = if (resCD) HttpEntity.CloseDelimited(ContentTypes.`text/plain(UTF-8)`,
Source.single(ByteString("ENTITY")))
else HttpEntity("ENTITY"), protocol = resProto),
requestMethod = if (headReq) HttpMethods.HEAD else HttpMethods.GET,
requestProtocol = reqProto,
closeAfterResponseCompletion = HttpMessage.connectionCloseExpected(reqProto, reqCH map (Connection(_)))) should renderTo(
expected = renCH match {
case Some(connection) ⇒
s"""HTTP/1.1 200 OK
|Server: akka-http/1.0.0
|Date: Thu, 25 Aug 2011 09:10:29 GMT
|Connection: $connection
|Content-Length: 0
|
|"""
case None ⇒
"""HTTP/1.1 200 OK
|Server: akka-http/1.0.0
|Date: Thu, 25 Aug 2011 09:10:29 GMT
|Content-Length: 0
|
|"""
}, close = close)
}
closeRequested = HttpMessage.connectionCloseExpected(reqProto, reqCH)) should renderTo(
s"""${resProto.value} 200 OK
|Server: akka-http/1.0.0
|Date: Thu, 25 Aug 2011 09:10:29 GMT
|${renCH.fold("")(_ + "\n")}Content-Type: text/plain; charset=UTF-8
|${if (resCD) "" else "Content-Length: 6\n"}
|${if (headReq) "" else "ENTITY"}""", close))
}
}

Expand Down

0 comments on commit 1b52ae3

Please sign in to comment.