Add support to forward grpc binary metadata #737
Conversation
timonwong
force-pushed
the
feature/binary-header
branch
from
1982907
to
63d5f8b
Compare
Codecov Report
@@ Coverage Diff @@
## master #737 +/- ##
=========================================
Coverage ? 55.86%
=========================================
Files ? 30
Lines ? 3170
Branches ? 0
=========================================
Hits ? 1771
Misses ? 1225
Partials ? 174
Continue to review full report at Codecov.
|
| pairs = append(pairs, "authorization", val) | ||
| } | ||
| if h, ok := mux.incomingHeaderMatcher(key); ok { | ||
| // Handles "-bin" metadata in grpc, since grpc will do another base64 | ||
| // encode before sending to server, we need to decode it first. | ||
| if strings.HasSuffix(key, metadataHeaderBinarySuffix) { |
There was a problem hiding this comment.
I don't quite understand the context here, but should there be a TrimSuffix somewhere? Or do we intentionally preserve the -bin?
There was a problem hiding this comment.
@rogerhub It's intentionally to preserve -bin suffix if we want to passing binary data, see https://github.com/grpc/grpc-go/blob/a91fb537b19866e5d1791fa1c6fb0530df26aadc/Documentation/grpc-metadata.md#storing-binary-data-in-metadata
runtime/context_test.go
Outdated
| @@ -9,6 +9,7 @@ import ( | |||
| "context" | |||
| "github.com/grpc-ecosystem/grpc-gateway/runtime" | |||
| "google.golang.org/grpc/metadata" | |||
| "encoding/base64" | |||
There was a problem hiding this comment.
Should this be in the previous block of imports? (to be consistent with runtime/context.go)
runtime/context_test.go
Outdated
| @@ -68,6 +69,30 @@ func TestAnnotateContext_ForwardsGrpcMetadata(t *testing.T) { | |||
| } | |||
| } | |||
|
|
|||
| func TestAnnotateContext_ForwardGrpcBinaryMetadata(t *testing.T) { | |||
| ctx := context.Background() | |||
| request ,err := http.NewRequest("GET", "http://www.example.com", nil) | |||
runtime/context_test.go
Outdated
| t.Errorf("Expected %d metadata items in context; got %v", emptyForwardMetaCount+1, md) | ||
| } | ||
| if got, want := md["test-bin"], []string{string(binData)}; !reflect.DeepEqual(got, want) { | ||
| t.Errorf(`md["grpcgateway-test-bin"] = %q want %q`, got, want) |
timonwong
force-pushed
the
feature/binary-header
branch
from
63d5f8b
to
400ef68
Compare
|
LGTM, thanks for your contribution! |
Fixes #218