Add support for http trailers and full range keys

[codefromthecrypt]

fasthttp implements trailing headers as a part of the http request and response headers objects. To ensure we can use these without relying on fasthttp API, we need a few more functions, which I'm happy to implement after #65 is in. I need this to help support the new http-wasm host, which has glitches both in trailers and also multiple header values.

• GetKeys and GetKeysTrailer, to return all keys including ones special-cased internally by fasthttp
  • this is much easier than having users do a range function, as well more precise to unit test
• GetTrailer to return a single trailer
• AddTrailer to add a single trailer field for a key
• SetTrailer to overwrite a trailer with a single value
• DelTrailer to delete all trailers for a key
• GetAll and GetAllTrailer function to return all values for a key
  • this is much easier than having users do a range function, as well more precise to unit test

cc @taoyuanyuan I'll backfill unit tests and polish up stuff, but probably this should be done separate from the version bump

[codefromthecrypt]

I referenced some upstream issues on fasthttp as at some point it would be nice for these to be in the standard library underneath as well. Regardless, we'll need apis here as otherwise when people access code, they are reliant on the implementation of fasthttp, not the types and functions defined explicitly here:

valyala/fasthttp#1400
valyala/fasthttp#1401

cc @li-jin-gou

[antJack]

mosn filter's API already provide accessibility to http header/body/trailer

func (f *filter) OnReceive(ctx context.Context, headers HeaderMap, body IoBuffer, trailers HeaderMap) StreamFilterStatus
                                                                                    ^^^
func (f *Filter) Append(ctx context.Context, headers api.HeaderMap, body buffer.IoBuffer, trailers api.HeaderMap) api.StreamFilterStatus
                                                                                             ^^^

you can simple put the header/body/trailer in the filter(ctx) and get them whenever you need

[codefromthecrypt]

@antJack ok I think there are a couple things then. one is that fasthttp doesn't work with trailers, yet. mosn/mosn#2145 (comment)

When we update here, we need a different type for trailers because in fasthttp trailers are in the same requestheaders type as headers. If we don't handle that here, then mosn will need similar logic to do it. valyala/fasthttp#1165

There's a separate topic about access for GetAllKeys and GetAll, but let's talk about ^^ first.

[antJack]

ok, after reading those materials, I probably understand what you want to do.

If there is any idea about where to add those trailers-related api? Another problem is that whether mosn should also "implements trailing headers as a part of the http request and response headers", since mosn's api is not just for http.

[codefromthecrypt]

My first thought was to add the extra apis to the existing http request/response types here https://github.com/mosn/pkg/blob/master/protocol/http/types.go

another way could be to define new types RequestTrailers and ResponseTrailers, if that's easier to integrate with the existing mosn callbacks. either way, it would be http only. wdyt?

[codefromthecrypt]

I will work on an implementation, as after we should cut a version of api, pkg so that mosn/holmes#129 and then eventually dapr can have api/pkg version alignment

[antJack]

We may prefer not changing the top-level stream filter api, such as:

func (f *filter) OnReceive(ctx context.Context, headers HeaderMap, body IoBuffer, trailers HeaderMap) StreamFilterStatus
func (f *Filter) Append(ctx context.Context, headers api.HeaderMap, body buffer.IoBuffer, trailers api.HeaderMap) api.StreamFilterStatus

just keeps the separation of headers/trailers, they were designed not only for http, and not even for fasthttp.

and it's ok to define new types RequestTrailers and ResponseTrailers in pkg/protocol/http, and finally integrate with fasthttp in pkg/stream/http

[codefromthecrypt]

sounds good. My plan was that even if the ResponseTrailers type is backed by fasthttp RequestHeaders one, the call sites don't change. I think we are eye-to-eye!

[codefromthecrypt]

@antJack there are a few glitches we'll run into with fasthttp because the trailers are comingled with the headers.

For example, logic like this either needs to be inaccurate or filter out the trailer keys

func (h ResponseHeader) ByteSize() (size uint64) {
	h.VisitAll(func(key, value []byte) {
		size += uint64(len(key) + len(value))
	})
	return size
}

Same issue in range really. I have no issue doing the filtering meanwhile (Ex. inspect to see if a header key is a trailing one and filter it out plus visa versa).

p.s. I'm not sure this is the right byte size? Is it supposed to be the text encoded size? if so, I think we need colons at least if not colon space? Maybe we can go back and document ByteSize more specifically? or should we just drop it.. wdyt?

[antJack]

just keep it simple, the most common usage scenarios of ByteSize() is to report size to logger or other data sinks. In these scenes, inaccuracy (not filtering out trailers) is better than missing them.

seems that byte size should account for colons, ref: https://github.com/envoyproxy/envoy/blob/main/source/common/http/header_map_impl.cc#L250

[codefromthecrypt]

thanks for the advice @antJack!

[antJack]

sorry i said it wrong yesterday

byte size should not account for colons

[codefromthecrypt]

@antJack I think maybe it is a good idea to release api and pkg just the version updates before I do this change. Then after we can follow-up and make improvements indepedent of go+fasthttp. wdyt?

[codefromthecrypt]

cc @taoyuanyuan on ^^ basically I think safest way is to tag api/pkg before I change any logic in it, mainly to adjust for vendoring go 1.18 and fasthttp version update. then roll that through, and after this go back and change logic.

[codefromthecrypt]

when next fasthttp comes out (v1.42.0), we can revisit this valyala/fasthttp#1405

[codefromthecrypt]

1.42 is out