Transfer-Encoding: chunked does not imply a body is present

[esmet]

Consider a service written with go-openapi receiving an HTTP request with the following attributes:

• GET method
• No content-type header
• No content-length header
• A transfer-encoding: chunked header
• A single, empty chunk (ie: the body transmitted is semantically empty, but does consume bytes on the HTTP body as is consistent with chunked encoding)

I see in practice that the code assumes the default content-type: application/octet-stream for this request because it believes there is a body. By assuming the default content type, service specs that do not explicitly support octet-stream will reject this request as invalid (HTTP 415). I believe the code responsible for deciding whether a request gets a content-type based on the presence of a request body is here: https://github.com/go-openapi/runtime/blob/master/request.go#L45

I've been struggling to definitively prove or disprove whether this behavior is strictly correct according to https://tools.ietf.org/html/rfc2616#section-14.17 and https://tools.ietf.org/html/rfc2616#section-14.41

Specifically, I interpret the RFC as saying:

• Transfer encoding and content length are mutually exclusivel. You can choose to use one or the other without changing the semantics of the message.
• Transfer encoding is a property of the message, not the entity.
• Content-Type is a property of the entity.

Reviewing https://github.com/go-openapi/runtime/blob/master/request.go#L45 it feels like..

// HasBody returns true if this method needs a content-type
func HasBody(r *http.Request) bool {
return len(r.TransferEncoding) > 0 || r.ContentLength > 0
}

..is suggesting that any request with a non-zero content length has a body. I believe this is correct. However, it also suggests that any request with a transfer-encoding header has a body and, according to the comment, "needs a content-type". Since the entity can still be empty (ie: a single empty chunk), and empty entities do not require a content-type, this seems wrong.

I'd like to start a discussion on whether this is a bug, or if there is additional nuance in the RFC or other practicalities that make the current behavior desirable.

[casualjim]

Without a content type how would you know what to do with the message?

[esmet]

My understanding is that content-type is not meaningful for requests that do not contain an entity body. I think a positive content-length header implies the existence of an entity body. In my interpretation of the RFC, the presence of a transfer-encoding header alone does not, but the code is using that to indicate that there is a body.

[casualjim]

Yes but if you set transfer encoding to chunked, aren't you implying there is going to be a message, and wouldn't that message then not need to be understood in some way?

[esmet]

It sounds like the RFC differentiates between a message and an entity body. From https://tools.ietf.org/html/rfc2616#section-4.3 (edit it seems https://tools.ietf.org/html/rfc7230#section-3.3.1 is the preferred RFC, according to the internet, and after a quick glance it looks like the below language is consistent)

"Transfer-Encoding MUST be used to indicate any transfer-codings applied by an application to ensure safe and proper transfer of the message."

"Transfer-Encoding is a property of the message, not of the entity, and thus MAY be added or removed by any application along the request/response chain."

My interpretation of the second sentence is that an empty message body may be represented as content-length: 0 or using chunked transfer encoding with an empty chunk. These both represent a 0-length entity body message.

This leads me to believe that https://github.com/go-openapi/runtime/blob/master/request.go#L45 is conflating the message with the entity body. What do you think?

[casualjim]

You can't interpret a http message as chunked because with what you're telling me it would be possible to know that a message is chunked before you got the headers, which is simply not true.

So if we accept that you have to be able to at least read the headers before you know what to expect from the message.Now what happens when we see transfer-coding chunked.

• We see that we will get a request of which the body has an unknown length (this can be 0 or more) but we're likely to have to at least look at it even to determine that it's empty.
• We also have to expect that the request doesn't end after the body closes, but instead anticipate that there might be a trailer to the request which should be announced via a headers (this is such a rare case that we currently don't support this)

Now if it holds true that we at least of the look at the message body on the server even to determine that it's empty, we need to have some way to interpret that body regardless of it being empty or not, correct?

[esmet]

I think we do not need to interpret (meaning, use a content-type on) the body unless it is non-empty. The important distinction is that, while the message is non-empty (ie: because it takes > 0 bytes to frame any chunked encoded body), the entity-body within the message is empty. RFC7231 (https://tools.ietf.org/html/rfc7231) suggests that a content-type should be sent when a message contains an entity body, but it doesn't say what needs to happen when a message does not contain an entity body. There is room for interpretation here.

   A sender that generates a message containing a payload body SHOULD
   generate a Content-Type header field in that message unless the
   intended media type of the enclosed representation is unknown to the
   sender.  If a Content-Type header field is not present, the recipient
   MAY either assume a media type of "application/octet-stream"
   ([RFC2046], Section 4.5.1) or examine the data to determine its type.

Taking a step back, it also looks like the RFC states that content-type applies to entity-bodies, not messages. On the other hand, transfer-encoding applies to messages and not entity-bodies. From this, my interpretation is that messages and entity-bodies are strictly separate, semantically. Abstractly, the message carries the body, which itself is zero or more bytes.

These are the cases for sending an empty entity body, that, in my opinion, illustrate things:

1. Content-Length is set and no transfer-encoding is set. The entity body exists if the content length is greater than zero.
2. Content-Length is not set and no transfer-encoding is set. The entity body exists if the bytes that follow the body are non-empty (not counting the end-of-body markers or trailers, for the sake of simplicity)
3. Content-Length is not set and a transfer-encoding is specified as "chunked". The entity body exists of the decoded body (ie: the unchunked body, after unapplying chunked encoding) has length greater than 0.

A common, clear case for 1) is when the method is GET and content-length is zero. Most HTTP implementations (including go-openapi's) take this to mean that there is no entity body. In theory, cases 2) and 3) are semantically equivalent to 1) and should therefore be treated the same way from a entity-body perspective.

I think this section of the RFC clarifies the semantics (https://tools.ietf.org/html/rfc7230#section-3.3.1):

   Unlike Content-Encoding (Section 3.1.2.1 of [RFC7231]),
   Transfer-Encoding is a property of the message, not of the
   representation, and any recipient along the request/response chain
   MAY decode the received transfer coding(s) or apply additional
   transfer coding(s) to the message body, assuming that corresponding
   changes are made to the Transfer-Encoding field-value.  Additional
   information about the encoding parameters can be provided by other
   header fields not defined by this specification.

This section allows recipients along the request/response chaing (eg: a reverse-proxy) to apply (or unapply) transfer codings to the message without changing the semantics of the entity body.

What is your opinion on my interpretation of section 3.3.1, the relationship between the three examples, and my claim that these are semantically equivalent, though not structurally equivalent?

[casualjim]

Yes I understand that you can interpret the spec that way, but in practice most people use the Transfer-Encoding to do unbounded streams of messages, like new line delimited json messages or SSE.

The fact that the transfer-encoding is about the entity allows for a de-chunking proxy to be in front and it can read the entire message, collect the body in some way and when it has read the trailers it can make use of the content length it received from the trailers. Bearing in mind that for it to have consumed the request body it needs to process the entire message body because the trailers only arrive in the end.
This proxy can then forward the request, moving the trailer headers to the prelude headers and then attaching the body (or no body), without actually changing the semantics of the request and the way it is handled by the upstream server.
I think this is not actually what people intend when they use transfer-encoding chunked in API's but they'd want to parse the stream somehow and give meaning to the chunks themselves so they can get a message stream.

What is the use case you want to address, that prompts this?

[esmet]

I started looking into this behavior after an Envoy proxy upgrade. Previously, on empty bodied GET requests with no explicit content-length header, envoy would proxy the request upstream and add content-length: 0 to the request. Since envoy noticed the client request was empty, it took the liberty to add an explicit content length header. This seems valid according to HTTP (example 1 vs example 2 in my previous comment).

After the upgrade, Envoy started sending transfer-encoding: chunked with a single empty chunk instead of setting content-length: 0 (example 3). This behavioral change effected some, but not all, of the upstream HTTP implementations that are part of the ecosystem that I manage. This is to say, some implementations were OK with this change, and some were not. At first I thought the change itself was pretty odd and possibly wrong, but after a lot of poking around I wasn't fully convinced that the change was actually semantically incorrect. It may be silly to chunk an empty body, but potentially not incorrect.

I ended up here because I noticed that, in particular, our services built on go-openapi were negatively effected. They started rejecting requests with HTTP 415 after assuming content-type: octet/stream for these seemingly empty bodied requests.

I identified several changes to overcome this:

• All clients set content-length: 0 for their empty bodied requests.
• All clients set content-type to a known valid value without setting content-length (interesting note: this confused some other libraries, unrelated to go-openapi, more fun to deal with)
• All upstream services accept application/octet-stream (bad solution)

Ultimately, I pose this question to the go-openapi maintainers first, before the Envoy maintainers, because I can't find any concrete evidence that says any of the above client workarounds are legitimately required. Most of the RFC quoting above shows that this behavioral change should, in theory, not require any client changes. I think the client request is valid, and the proxy's transformation is valid (though silly). I'm currently stuck on the way this is interpreted by go-openapi.

[esmet]

For added context, here is the Envoy code that decides whether to send content-length: 0 or to assume chunked encoding upstream: https://github.com/envoyproxy/envoy/blob/master/source/common/http/http1/codec_impl.cc#L85

[casualjim]

I see, thanks for all the questions so far and the extra context. Implementing http properly always seems like it's easy but it's been a journey for me so far.

We can further refine how we handle requests with Transfer-Encoding so that we have proper behavior for this case. Envoy is common enough for me to want to play nice with it.

To summarize the expected behavior:

• We assume that there is a body when there is a non zero content length (exists today)
• We suspect there might be a body when there is no content-length set
  • when there is no transfer-encoding set look at the first byte of the stream and if it's not EOF we have a body
  • when there is a transfer-encoding chunked set, look at the first byte of the stream and if it's not EOF we have a body

do you agree? I'll make the necessary changes for this.

[casualjim]

It does leave something unclear though because how would we need to interpret the consumes from the swagger spec.

when they are omitted, it's clear what would need to happen, we can safely bypass validation. But when they are present then we currently make that a strict requirement.

when no body is present we can also safely ignore the validation

when a body is present we will require a content type and it needs to be strict otherwise we'd have no way of knowing how to interpret the request.

[esmet]

The expected behavior you summarized sounds correct, with a few clarifications/points:

• when there is no transfer-encoding set look at the first byte of the stream and if it's not EOF we have a body

Agreed. If there is no encoding, then we know the entity body is terminated by the CRLF ending (right?) and that's all you need to check for.

• "when there is a transfer-encoding chunked set, look at the first byte of the stream and if it's not EOF we have a body":

I want to clarify that the case in question sends a single empty chunk after headers to denote an "empty body". If I recall correctly, a single empty chunk is represented as '\r\n\r\n0\rn\n' (two CRLF to end headers, one ascii zero, two CRLF to terminate). I think as long as this statement means "look at the first byte of the decoded first chunk and check for empty" we should be ok.

Back to the original behavior from Envoy after upgrade:

I'm still going to bring this up with the Envoy team, since I don't think this behavior is actually expected from the code. It only happens when using Envoy with multiple HTTP filters (such as an authorization and rate_limit filter, common use case with www.getambassador.io). My guess is that they will say it's a result of some nuanced interaction between components in their code and technically valid, so a fix, if any, won't be high priority.

Thanks for helping me dig into this so far.