Should metadata annotator include the headers from incoming matcher?

[tamalsaha]

@tmc https://github.com/grpc-ecosystem/grpc-gateway/blob/master/runtime/context.go#L97

Currently metadataAnnotator is passed the original ctx which does not include the metadata / headers. We can consider passing a context with the headers to metadata annotator. This will allow custom logic inside metadata annotator that depends on the http headers.

cc: @ilius

[tamalsaha]

To be clear, any custom logic in metadata annotator can be also performed on the grpc server side after passing the full header. An example can be adding a new header x-cookie-key1 by parsing the Cookie header.

[tamalsaha]

On second thought, this is not needed. Custom logic can read the original req.

[ilius]

I'm not sure it's related to this, but I think we should be able to add one specific cookie (or all of them) to the incoming metadata.MD of context, so that we don't have to iterate over md["cookie"], every time.
Specially inside the grpc ecosystem (where there is no http1/REST) in micro-service architecture, we frequently set and get a specific metadata key, and we don't want to iterate over md["cookie"] or append to it (or both) every time (that's kind of stupid). Even the term cookie should not be used when you are inside grpc ecosystem. Cookie is for HTTP1 / REST, not GRPC.

[tamalsaha]

@ilius , Cookie is a valid concept in both Http and Http/2 (including gRPC). IMO, your server should work whether the cookie comes via Http or Http/2. If you want to parse cookie on the gRPC side, this is what I do:

• Create a http.Request from ctx. e.g. Support for easier parsing and creation of Cookie grpc/grpc-go#1174 (comment)
• Then use req.Cookie("") for parsing

But if you want to only support cookie at gateway layer, this is what you can do:

runtime.NewServeMux(runtime.WithMetadata(func(ctx context.Context, req *http.Request) (metdata.MD) {
	return metdata.Pairs("x-cookie-k1", req.Cookie("k1"))
}));

[ilius]

We don't expose grpc to the outside world (only for inter-service communication), we only expose REST to outside. So I will try the second approach.

[ilius]

The second approach does not seem to be working for us.

s.grpcMux = runtime.NewServeMux(runtime.WithMetadata(func(ctx context.Context, req *http.Request) metadata.MD {
	tokenCookie, _ := req.Cookie("token")
	if tokenCookie == nil {
		return nil // metadata.Pairs()
	}
	fmt.Printf("WithMetadata: tokenCookie: %#v\n", tokenCookie) // correctly set
	return metadata.Pairs("token", tokenCookie.Raw)
}))

But I get this metadata:

md=metadata.MD{"token":[]string{""}, "grpcgateway-cookie":[]string{"token=......."}, ...}

It gives empty "token":[]string{""}

[ilius]

I also tried with both ServeMuxOptions to NewServeMux, WithMetadata only add an empty context key.

[tmc]

@ilius are you seeing that behavior if you supply a literal value instead of "tokenCookie.Raw" there in your example?

[tamalsaha]

@ilius , you should be using Cookie.String(), not Cookie.Raw. From https://golang.org/src/net/http/cookie.go, you can see that Cookie.Raw is set for Set-Cookie header, but there is no Set-Cookie header in http request.

[ilius]

My apology.
I should have used cookie.Value
Thanks

[tamalsaha]

Glad it worked for you!