Connection: close breaks streaming responses

[zhxiaogg]

Hi, below is the complete unit test I used, and it will success if I replace the request protocol from HTTP 1.0 to HTTP 1.1. See also https://groups.google.com/forum/#!topic/akka-user/wa5UIB95SWA with detail log.

// setup mock server
val json =
  """
    |{"result":true}
  """.stripMargin

val source = Source(ByteString(json) :: Nil)
val mockRoutes = path("test" / "proxy_chunked.do") {
  get {
    complete {
      HttpResponse(StatusCodes.OK, entity = HttpEntity(ContentTypes.`application/json`, source))
    }
  }
} ~ complete {
  HttpResponse(StatusCodes.NotFound)
}

val (_, host, port) = AkkaHttpTestUtils.temporaryServerHostnameAndPort()
val bindingFuture = Http().bindAndHandle(mockRoutes, host, port)

val request = HttpRequest(
  HttpMethods.GET,
  uri = s"http://${host}:${port}/test/proxy_chunked.do",
  protocol = HttpProtocols.`HTTP/1.0`
)

val futureByteString = Http().singleRequest(request).map { resp =>
  println("response returned")
  resp.entity.dataBytes
} flatMap { source =>
  source.runFold(ByteString.empty)((r, bs) => r ++ bs)
}
val byteString = Await.result(futureByteString, 10 seconds)
byteString.utf8String shouldEqual (json)

[viktorklang]

Http 1.0 doesn't support transfer-encoding chunked?
http://www8.org/w8-papers/5c-protocols/key/key.html

[aruediger]

Update: I was wrong. Here is the response

Content-Type:application/json
Date:Tue, 05 Apr 2016 12:58:11 GMT
Server:akka-http/2.4-SNAPSHOT
Transfer-Encoding:chunked

@viktorklang Looks like this isn't chunked but CloseDelimited i.e. the end of the entity is signaled by closing of the connection which seems to terminate the selector before the stream is materialized.

HttpResponse(200 OK,List(Server: akka-http/2.4-SNAPSHOT, Date: Tue, 05 Apr 2016 12:39:17 GMT, Connection: close),HttpEntity.CloseDelimited(application/json,Source(SourceShape(StreamUtils$$anon$2.out), CompositeModule [1451cc3d]
  Name: unnamed
  Modules:
    (unnamed) CompositeModule [44de6922]
      Name: unnamed
      Modules:
        (SubSource%28EntitySource%29) GraphStage(EntitySource) [6c8461f5]
        (unnamed) [15ce6519] copy of GraphStage(Collect) [0f6463b9]
        (limitable) [38c48c0e] copy of CompositeModule [7780c92a]
          Name: limitable
          Modules:
            (unnamed) GraphStage(unknown-operation) [11809c6e]
          Downstreams: 
          Upstreams: 
          MatValue: Ignore
      Downstreams: 
        SubSource.out -> Collect.in
        Collect.out -> unknown-operation.in
      Upstreams: 
        Collect.in -> SubSource.out
        unknown-operation.in -> Collect.out
      MatValue: Atomic(SubSource%28EntitySource%29[6c8461f5])
    (unnamed) [3dd54c4c] copy of GraphStage(akka.http.impl.util.StreamUtils$$anon$2@5055479a) [216d1304]
  Downstreams: 
    SubSource.out -> Collect.in
    Collect.out -> unknown-operation.in
    unknown-operation.out -> StreamUtils$$anon$2.in
  Upstreams: 
    Collect.in -> SubSource.out
    unknown-operation.in -> Collect.out
    StreamUtils$$anon$2.in -> unknown-operation.out
  MatValue: Atomic(akka.stream.impl.StreamLayout$CompositeModule[44de6922]))),HttpProtocol(HTTP/1.1))

[viktorklang]

@2Beaucoup I should've know better than to trust the title of the Issue… :)

If it is as you say, then I guess the problem is that the short-circuit is too eager and needs to wait for completion before tearing down the connection.

[zhxiaogg]

@viktorklang I really shouldn't use the word chunked just because the streaming response entity. Sorry for that.

So, the reason why server returns a CloseDelimited response is because I send a HTTP 1.0 request, right? I can by pass this problem if I send all request with HTTP 1.1 protocol, can you confirm this?

[viktorklang]

@zhxiaogg No worries, I was only joking :)

[aruediger]

@viktorklang please see my update above. The response is indeed chunked as a result of using the Entity constructor with Source[ByteString]. The response is HTTP/1.1 so chunking should be fine.

The problem is the connection closing and the failure also happens with HTTP 1.1 if an Connection: close is added to the response.

[viktorklang]

@2Beaucoup I'm surprised that there is no test which verifies this behavior.

[aruediger]

@viktorklang Me too! ;) I added one here.

[aruediger]

Problem seems to be this line where the stage gets completed before the substream is completed.

/cc @sirthias

[sirthias]

If the client signals only HTTP/1.0 capabilities the server is actually not allowed to respond with an HTTP/1.1 response.

So, the reason why server returns a CloseDelimited response is because I send a HTTP 1.0 request, right?

Yes.

I can by pass this problem if I send all request with HTTP 1.1 protocol, can you confirm this?

Yes.

[aruediger]

If the client signals only HTTP/1.0 capabilities the server is actually not allowed to respond with an HTTP/1.1 response.

https://tools.ietf.org/html/rfc2145#section-2.3 suggests otherwise:

An HTTP server SHOULD send a response version equal to the highest version for which the server is at least conditionally compliant, and whose major version is less than or equal to the one received in the request.

Question is: Should the server reply with Transfer-Encoding: chunked if the request was HTTP/1.0?

[viktorklang]

@2Beaucoup How could it? chunked belongs to the 1.1 spec, and a client communicating using 1.0 shouldn't be expected to understand a newer protocol specification.

[sirthias]

Yes, true. My statement from before was misleading.
If the server receives an HTTP/1.0 request it should signal its own capabilities but not present the client with a response that it must assume the client to not be able to properly interpret.
So the right approach would be to send a response with protocol marker HTTP/1.1 but NOT use Transfer-Encoding: chunked (but rather close-after-response-end).

[aruediger]

I can by pass this problem if I send all request with HTTP 1.1 protocol, can you confirm this?

Yes.

See the test case linked above. It doesn't work for HTTP 1.1 as well, when sent with Connection: close as the PrepareResponse stage seems to get completed before the chunk substream.

[sirthias]

@2Beaucoup Yes, there may be a problem in the current implementation. I was thinking about the what the right behavior should be.
It really is quite easy to misinterpret the protocol marker of an HTTP response. It does not qualify the response it is part of but rather the capabilities of the server. I forget about this myself all the same.

[aruediger]

So the right approach would be to send a response with protocol marker HTTP/1.1 but NOT use Transfer-Encoding: chunked (but rather close-after-response-end).

Makes sense. I opened a new ticket. Update: It's already implemented that way.

[aruediger]

Sorry for leading the discussion in the wrong direction.

The issue here is solely that the response entity substream can't be consumed if Conection: close is set. (See my two comments from above.)

[sirthias]

@2Beaucoup No prob at all. Thanks for digging through this!

[aruediger]

No prob @sirthias.

FTR: The problematic change seems to be e3ee285#diff-9d641a9b26bb92cd487f4e6c81832424R186.