Sending multiple headers in Go HTTP filter with FilterCallbacks.SendLocalReply

[michaelsauter]

Title: How to send multiple headers in Go HTTP filter with FilterCallbacks.SendLocalReply

Description:
I'm implementing a Go HTTP filter and I am trying to send multiple Set-Cookie headers. However, the type of the headers parameter in the signature of

envoy/contrib/golang/common/go/api/filter.go

Line 155 in 75b5061

SendLocalReply(responseCode int, bodyText string, headers map[string]string, grpcStatus int64, details string)

is a map[string]string so I can't repeat the same header. In the specific case of cookies, it may be possible to separate multiple cookies by comma, but this is discouraged by RFC6265, and in my tests it didn't work. After some trial and error I found a working solution:

f.callbacks.SendLocalReply(http.StatusOK, "body", map[string]string{
	"Set-Cookie": "one=a\nSet-Cookie: two=b\nSet-Cookie: three=c",
}, 0, "")

However, that looks rather hacky and I wonder if this is how it is supposed to be? Is there a reason the type of headers isn't map[string][]string in line with https://pkg.go.dev/net/http#Header? Or should this be considered a bug?

[yanavlasov]

Adding extension owners: @doujiang24 @wangfakang @StarryVae @spacewander @antJack

[spacewander]

@doujiang24 and I discussed this months ago. We have a plan to add multiple headers support. But this is still a TODO as we don't have bandwidth to do it.

@michaelsauter
Would you give it a try? You may need to update https://github.com/envoyproxy/envoy/blob/f97242a970eb6637b2aa8bba916f589672a1d190/contrib/golang/filters/http/source/cgo.cc#L80C33-L80C33

[michaelsauter]

@spacewander I can give it a try, but it might take some time. Thanks for the pointer to the source file that likely needs changing. So just to be sure we are aligned - your intention is to have the signature updated to the following:

SendLocalReply(responseCode int, bodyText string, headers map[string][]string, grpcStatus int64, details string) 

Correct?

[spacewander]

@spacewander I can give it a try, but it might take some time. Thanks for the pointer to the source file that likely needs changing. So just to be sure we are aligned - your intention is to have the signature updated to the following:

SendLocalReply(responseCode int, bodyText string, headers map[string][]string, grpcStatus int64, details string) 

Correct?

Yes.
You may need to update:

return envoyGoFilterHandlerWrapper(
      r,
      [response_code, body_text_data, body_text_len, headers, headers_num, grpc_status,
       details_data, details_len](std::shared_ptr<Filter>& filter) -> CAPIStatus {
        auto header_values = stringsFromGoSlice(headers, headers_num);
        std::function<void(Http::ResponseHeaderMap&)> modify_headers =
            [header_values](Http::ResponseHeaderMap& headers) -> void {
          for (size_t i = 0; i < header_values.size(); i += 2) {
            const auto& key = header_values[i];
            const auto& value = header_values[i + 1];
            if (value.length() > 0) {
              headers.setCopy(Http::LowerCaseString(key), value); // change this to append. Maybe work, I haven't tried
            }
          }

[doujiang24]

yep, headers.addCopy() should works here.

[michaelsauter]

Sorry for not getting back earlier, I am currently unable to drive this further.

I pushed what I did so far here: michaelsauter@2e1c769. I did not manage to add a test / run the tests yet because of an error (it seems to be the same as bazel-contrib/rules_foreign_cc#859). As this is my first time using Bazel / C++ it is not clear for me how to proceed and right now unfortunately I lack the time to look further into it :(

[doujiang24]

@michaelsauter Thanks for your try, that's ok, I'll create a PR for it soon.