Envoy: Lua EnvoyFilter set multiple set-cookie header in request_handle functions

[fentas]

Title: Set mutiple Set-Cookie header within a Lua EnvoyFilter as reponse
_istio verision:_ v1.2.2

I tried to concat with , like

headers["set-cookie"] = "test=123; Path=/, foo=bar; Path=/" 

This will be send but not unfolded.

Also tied using a table (like Nginx does it) no luck -> got table, sting expected.

[mattklein123]

I believe if you use the add() Lua API on the headers and add individual set-cookie headers it will work.

[fentas]

Can not do. Using httpCall and respond
https://www.envoyproxy.io/docs/envoy/latest/configuration/http_filters/lua_filter#httpcall
https://www.envoyproxy.io/docs/envoy/latest/configuration/http_filters/lua_filter#respond

  -- Make an HTTP call to an upstream host with the following headers, body, and timeout.
  local headers, body = request_handle:httpCall(
  "lua_cluster",
  {
    [":method"] = "POST",
    [":path"] = "/",
    [":authority"] = "lua_cluster",
    ["Set-Cookie"] = "test=123; Path=/",
    ["Set-Cookie"] = "foo=bar; Path=/"
  },
  "hello world",
  5000)

This will not work as httpCall accepts only a table of key/value pairs. Same for request_handle:respond.
If I could initialize a header object and use it instead, this would be perfect.

[mattklein123]

I see, OK. Marking enhancement / help wanted.

[dio]

@fentas, sorry to clarify, so you want to have a prepared headers object (so you can :add() or :set() etc) and be able to send as an argument for both request_handle:httpCall() and request_handle:respond()?

[fentas]

headers object would be one solution, and maybe the most elegant as this object exists already and can handle this (would be also more consistent).
Another would be that header could except also a table as value (like nginx lua does it).

The end goal would be that request_handle:httpCall and request_handle:respond could handle duplicate keys like Set-Cookie.

[jstewmon]

I've made the following related observations:

1. It appears impossible to receive multiple values with the same header (e.g. set-cookie) from httpCall b/c the headers result is a table of strings.
2. The handle:headers():get(name) method can only be used to access the first value of the given header name. To access all values of a header by name, one must iterate pairs(handle:headers()) and filter by key.

[jstewmon]

I was looking around to see if there is any other known, valid case for multiple HTTP headers with the same name, and set-cookie is the only example I can find. It's actually mentioned in the [HTTP/1.1 Spec].

I think a reasonable path forward would be to have special handling of the set-cookie header (this is what Node.js does).

For all cases where set-cookie is a key, the value can always be a lua table or nil (string could be supported for backward compatibility), rather than a string or nil.

headers, body = httpCall(
  "echo-cluster",
  { ..., "set-cookie" = {"foo=bar", "bar=baz"} },
  nil,
  1000
)
headers -- =>  { ..., "set-cookie" = {"foo=bar", "bar=baz"} }
for _, cookie in pairs(headers["set-cookie"]) do
  handle:headers():add("set-cookie", cookie)
end
handle:headers():get("set-cookie") -- => {"foo=bar", "bar=baz"}

The downside is that there's a special case to be handled for set-cookie in a few places, but the upside is that the interface remains compatible with the current release, and maintains the existing ergonomics. Having to build the headers imperatively with the :add() method would be less convenient IMO.

[fentas]

yeah I believe there is only one meaningful header with duplicate keys. Less meaningful headers are e.g. Warning but these ones will be or already are deprecated.

[mattklein123]

@jstewmon that sounds like a reasonable solution to me. I don't think it would be that hard to implement either. IMO you could actually just generically support this in the Lua code and if given a table for a header key, all of the table values can be appended.

[dio]

@fentas @jstewmon WDYT of having this line in here: https://github.com/envoyproxy/envoy/pull/7851/files#diff-b6a07032ffa44c0345fae0a39b2adc13R748 (so you're able to set a header entry with a table of strings). And the corresponding serialized header, if it is a predefined header like set-cookie is just like: https://github.com/envoyproxy/envoy/pull/7851/files#diff-b6a07032ffa44c0345fae0a39b2adc13R775

[jstewmon]

@dio the entry-with-table part looks fine - it's the same as suggested here, right? But, set-cookie headers can't be folded together into a single header: https://tools.ietf.org/html/rfc6265#section-3

[dio]

@jstewmon thank you for pointing it out. I'll revert it.

Proposed to fix it in: https://github.com/envoyproxy/envoy/pull/7851