grpc
/
grpc-go
mirror of https://github.com/grpc/grpc-go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Never encode binary metadata within the metadata map (#1188) 

This change ensures consistency for the user when accessing metadata values:
they are never encoded except when sent on the wire.  Previously, they would
appear encoded to client code, but not to server code.  As such, this
represents a behavior change, but one unlikely to affect user code, as it's
unusual to inspect the metadata after setting it.
This commit is contained in:
dfawley 2017-04-28 10:56:58 -07:00 committed by GitHub
parent 277e90a432
commit b610ffd3f8
8 changed files with 113 additions and 127 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
metadata/metadata.go
View File

@ -36,46 +36,15 @@
package metadata // import "google.golang.org/grpc/metadata"
import (
"encoding/base64"
"fmt"
"strings"
"golang.org/x/net/context"
)
const (
binHdrSuffix = "-bin"
)
// encodeKeyValue encodes key and value qualified for transmission via gRPC.
// Transmitting binary headers violates HTTP/2 spec.
// TODO(zhaoq): Maybe check if k is ASCII also.
func encodeKeyValue(k, v string) (string, string) {
k = strings.ToLower(k)
if strings.HasSuffix(k, binHdrSuffix) {
val := base64.StdEncoding.EncodeToString([]byte(v))
v = string(val)
}
return k, v
}
// DecodeKeyValue returns the original key and value corresponding to the
// encoded data in k, v.
// If k is a binary header and v contains comma, v is split on comma before decoded,
// and the decoded v will be joined with comma before returned.
// DecodeKeyValue returns k, v, nil. It is deprecated and should not be used.
func DecodeKeyValue(k, v string) (string, string, error) {
if !strings.HasSuffix(k, binHdrSuffix) {
return k, v, nil
}
vvs := strings.Split(v, ",")
for i, vv := range vvs {
val, err := base64.StdEncoding.DecodeString(vv)
if err != nil {
return "", "", err
}
vvs[i] = string(val)
}
return k, strings.Join(vvs, ","), nil
return k, v, nil
}
// MD is a mapping from metadata keys to values. Users should use the following
@ -83,11 +52,11 @@ func DecodeKeyValue(k, v string) (string, string, error) {
type MD map[string][]string
// New creates a MD from given key-value map.
// Keys are automatically converted to lowercase. And for keys having "-bin" as suffix, their values will be applied Base64 encoding.
// Keys are automatically converted to lowercase.
func New(m map[string]string) MD {
md := MD{}
for k, v := range m {
key, val := encodeKeyValue(k, v)
for k, val := range m {
key := strings.ToLower(k)
md[key] = append(md[key], val)
}
return md
@ -95,20 +64,19 @@ func New(m map[string]string) MD {
// Pairs returns an MD formed by the mapping of key, value ...
// Pairs panics if len(kv) is odd.
// Keys are automatically converted to lowercase. And for keys having "-bin" as suffix, their values will be appplied Base64 encoding.
// Keys are automatically converted to lowercase.
func Pairs(kv ...string) MD {
if len(kv)%2 == 1 {
panic(fmt.Sprintf("metadata: Pairs got the odd number of input pairs for metadata: %d", len(kv)))
}
md := MD{}
var k string
var key string
for i, s := range kv {
if i%2 == 0 {
k = s
key = strings.ToLower(s)
continue
}
key, val := encodeKeyValue(k, s)
md[key] = append(md[key], val)
md[key] = append(md[key], s)
}
return md
}

59
metadata/metadata_test.go
View File

@ -38,73 +38,20 @@ import (
"testing"
)
const binaryValue = string(128)
func TestEncodeKeyValue(t *testing.T) {
for _, test := range []struct {
// input
kin string
vin string
// output
kout string
vout string
}{
{"key", "abc", "key", "abc"},
{"KEY", "abc", "key", "abc"},
{"key-bin", "abc", "key-bin", "YWJj"},
{"key-bin", binaryValue, "key-bin", "woA="},
} {
k, v := encodeKeyValue(test.kin, test.vin)
if k != test.kout || !reflect.DeepEqual(v, test.vout) {
t.Fatalf("encodeKeyValue(%q, %q) = %q, %q, want %q, %q", test.kin, test.vin, k, v, test.kout, test.vout)
}
}
}
func TestDecodeKeyValue(t *testing.T) {
for _, test := range []struct {
// input
kin string
vin string
// output
kout string
vout string
err error
}{
{"a", "abc", "a", "abc", nil},
{"key-bin", "Zm9vAGJhcg==", "key-bin", "foo\x00bar", nil},
{"key-bin", "woA=", "key-bin", binaryValue, nil},
{"a", "abc,efg", "a", "abc,efg", nil},
{"key-bin", "Zm9vAGJhcg==,Zm9vAGJhcg==", "key-bin", "foo\x00bar,foo\x00bar", nil},
} {
k, v, err := DecodeKeyValue(test.kin, test.vin)
if k != test.kout || !reflect.DeepEqual(v, test.vout) || !reflect.DeepEqual(err, test.err) {
t.Fatalf("DecodeKeyValue(%q, %q) = %q, %q, %v, want %q, %q, %v", test.kin, test.vin, k, v, err, test.kout, test.vout, test.err)
}
}
}
func TestPairsMD(t *testing.T) {
for _, test := range []struct {
// input
kv []string
// output
md MD
size int
md MD
}{
{[]string{}, MD{}, 0},
{[]string{"k1", "v1", "k2-bin", binaryValue}, New(map[string]string{
"k1": "v1",
"k2-bin": binaryValue,
}), 2},
{[]string{}, MD{}},
{[]string{"k1", "v1", "k1", "v2"}, MD{"k1": []string{"v1", "v2"}}},
} {
md := Pairs(test.kv...)
if !reflect.DeepEqual(md, test.md) {
t.Fatalf("Pairs(%v) = %v, want %v", test.kv, md, test.md)
}
if md.Len() != test.size {
t.Fatalf("Pairs(%v) generates md of size %d, want %d", test.kv, md.Len(), test.size)
}
}
}

10
test/end2end_test.go
View File

@ -75,8 +75,9 @@ import (
var (
// For headers:
testMetadata = metadata.MD{
"key1": []string{"value1"},
"key2": []string{"value2"},
"key1": []string{"value1"},
"key2": []string{"value2"},
"key3-bin": []string{"binvalue1", string([]byte{1, 2, 3})},
}
testMetadata2 = metadata.MD{
"key1": []string{"value12"},
@ -84,8 +85,9 @@ var (
}
// For trailers:
testTrailerMetadata = metadata.MD{
"tkey1": []string{"trailerValue1"},
"tkey2": []string{"trailerValue2"},
"tkey1": []string{"trailerValue1"},
"tkey2": []string{"trailerValue2"},
"tkey3-bin": []string{"trailerbinvalue1", string([]byte{3, 2, 1})},
}
testTrailerMetadata2 = metadata.MD{
"tkey1": []string{"trailerValue12"},

12
transport/handler_server.go
View File

@ -111,6 +111,10 @@ func NewServerHandlerTransport(w http.ResponseWriter, r *http.Request) (ServerTr
v = v[:i]
}
}
v, err := decodeMetadataHeader(k, v)
if err != nil {
return nil, streamErrorf(codes.InvalidArgument, "malformed binary metadata: %v", err)
}
metakv = append(metakv, k, v)
}
}
@ -207,10 +211,9 @@ func (ht *serverHandlerTransport) WriteStatus(s *Stream, st *status.Status) erro
continue
}
for _, v := range vv {
// http2 ResponseWriter mechanism to
// send undeclared Trailers after the
// headers have possibly been written.
h.Add(http2.TrailerPrefix+k, v)
// http2 ResponseWriter mechanism to send undeclared Trailers after
// the headers have possibly been written.
h.Add(http2.TrailerPrefix+k, encodeMetadataHeader(k, v))
}
}
}
@ -265,6 +268,7 @@ func (ht *serverHandlerTransport) WriteHeader(s *Stream, md metadata.MD) error {
continue
}
for _, v := range vv {
v = encodeMetadataHeader(k, v)
h.Add(k, v)
}
}

12
transport/http2_client.go
View File

@ -437,23 +437,23 @@ func (t *http2Client) NewStream(ctx context.Context, callHdr *CallHdr) (_ *Strea
)
if md, ok := metadata.FromOutgoingContext(ctx); ok {
hasMD = true
for k, v := range md {
for k, vv := range md {
// HTTP doesn't allow you to set pseudoheaders after non pseudoheaders were set.
if isReservedHeader(k) {
continue
}
for _, entry := range v {
t.hEnc.WriteField(hpack.HeaderField{Name: k, Value: entry})
for _, v := range vv {
t.hEnc.WriteField(hpack.HeaderField{Name: k, Value: encodeMetadataHeader(k, v)})
}
}
}
if md, ok := t.md.(*metadata.MD); ok {
for k, v := range *md {
for k, vv := range *md {
if isReservedHeader(k) {
continue
}
for _, entry := range v {
t.hEnc.WriteField(hpack.HeaderField{Name: k, Value: entry})
for _, v := range vv {
t.hEnc.WriteField(hpack.HeaderField{Name: k, Value: encodeMetadataHeader(k, v)})
}
}
}

18
transport/http2_server.go
View File

@ -644,13 +644,13 @@ func (t *http2Server) WriteHeader(s *Stream, md metadata.MD) error {
if s.sendCompress != "" {
t.hEnc.WriteField(hpack.HeaderField{Name: "grpc-encoding", Value: s.sendCompress})
}
for k, v := range md {
for k, vv := range md {
if isReservedHeader(k) {
// Clients don't tolerate reading restricted headers after some non restricted ones were sent.
continue
}
for _, entry := range v {
t.hEnc.WriteField(hpack.HeaderField{Name: k, Value: entry})
for _, v := range vv {
t.hEnc.WriteField(hpack.HeaderField{Name: k, Value: encodeMetadataHeader(k, v)})
}
}
bufLen := t.hBuf.Len()
@ -713,21 +713,17 @@ func (t *http2Server) WriteStatus(s *Stream, st *status.Status) error {
panic(err)
}
for k, v := range metadata.New(map[string]string{"grpc-status-details-bin": (string)(stBytes)}) {
for _, entry := range v {
t.hEnc.WriteField(hpack.HeaderField{Name: k, Value: entry})
}
}
t.hEnc.WriteField(hpack.HeaderField{Name: "grpc-status-details-bin", Value: encodeBinHeader(stBytes)})
}
// Attach the trailer metadata.
for k, v := range s.trailer {
for k, vv := range s.trailer {
// Clients don't tolerate reading restricted headers after some non restricted ones were sent.
if isReservedHeader(k) {
continue
}
for _, entry := range v {
t.hEnc.WriteField(hpack.HeaderField{Name: k, Value: entry})
for _, v := range vv {
t.hEnc.WriteField(hpack.HeaderField{Name: k, Value: encodeMetadataHeader(k, v)})
}
}
bufLen := t.hBuf.Len()

35
transport/http_util.go
View File

@ -36,6 +36,7 @@ package transport
import (
"bufio"
"bytes"
"encoding/base64"
"fmt"
"io"
"net"
@ -50,7 +51,6 @@ import (
spb "google.golang.org/genproto/googleapis/rpc/status"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/grpclog"
"google.golang.org/grpc/metadata"
"google.golang.org/grpc/status"
)
@ -164,6 +164,31 @@ func (d *decodeState) status() *status.Status {
return d.statusGen
}
const binHdrSuffix = "-bin"
func encodeBinHeader(v []byte) string {
return base64.StdEncoding.EncodeToString(v)
}
func decodeBinHeader(v string) ([]byte, error) {
return base64.StdEncoding.DecodeString(v)
}
func encodeMetadataHeader(k, v string) string {
if strings.HasSuffix(k, binHdrSuffix) {
return encodeBinHeader(([]byte)(v))
}
return v
}
func decodeMetadataHeader(k, v string) (string, error) {
if strings.HasSuffix(k, binHdrSuffix) {
b, err := decodeBinHeader(v)
return string(b), err
}
return v, nil
}
func (d *decodeState) processHeaderField(f hpack.HeaderField) error {
switch f.Name {
case "content-type":
@ -181,12 +206,12 @@ func (d *decodeState) processHeaderField(f hpack.HeaderField) error {
case "grpc-message":
d.rawStatusMsg = decodeGrpcMessage(f.Value)
case "grpc-status-details-bin":
_, v, err := metadata.DecodeKeyValue("grpc-status-details-bin", f.Value)
v, err := decodeBinHeader(f.Value)
if err != nil {
return streamErrorf(codes.Internal, "transport: malformed grpc-status-details-bin: %v", err)
}
s := &spb.Status{}
if err := proto.Unmarshal([]byte(v), s); err != nil {
if err := proto.Unmarshal(v, s); err != nil {
return streamErrorf(codes.Internal, "transport: malformed grpc-status-details-bin: %v", err)
}
d.statusGen = status.FromProto(s)
@ -203,12 +228,12 @@ func (d *decodeState) processHeaderField(f hpack.HeaderField) error {
if d.mdata == nil {
d.mdata = make(map[string][]string)
}
k, v, err := metadata.DecodeKeyValue(f.Name, f.Value)
v, err := decodeMetadataHeader(f.Name, f.Value)
if err != nil {
grpclog.Printf("Failed to decode (%q, %q): %v", f.Name, f.Value, err)
return nil
}
d.mdata[k] = append(d.mdata[k], v)
d.mdata[f.Name] = append(d.mdata[f.Name], v)
}
}
return nil

44
transport/http_util_test.go
View File

@ -35,6 +35,7 @@ package transport
import (
"fmt"
"reflect"
"testing"
"time"
)
@ -143,3 +144,46 @@ func TestDecodeGrpcMessage(t *testing.T) {
}
}
}
const binaryValue = string(128)
func TestEncodeMetadataHeader(t *testing.T) {
for _, test := range []struct {
// input
kin string
vin string
// output
vout string
}{
{"key", "abc", "abc"},
{"KEY", "abc", "abc"},
{"key-bin", "abc", "YWJj"},
{"key-bin", binaryValue, "woA="},
} {
v := encodeMetadataHeader(test.kin, test.vin)
if !reflect.DeepEqual(v, test.vout) {
t.Fatalf("encodeMetadataHeader(%q, %q) = %q, want %q", test.kin, test.vin, v, test.vout)
}
}
}
func TestDecodeMetadataHeader(t *testing.T) {
for _, test := range []struct {
// input
kin string
vin string
// output
vout string
err error
}{
{"a", "abc", "abc", nil},
{"key-bin", "Zm9vAGJhcg==", "foo\x00bar", nil},
{"key-bin", "woA=", binaryValue, nil},
{"a", "abc,efg", "abc,efg", nil},
} {
v, err := decodeMetadataHeader(test.kin, test.vin)
if !reflect.DeepEqual(v, test.vout) || !reflect.DeepEqual(err, test.err) {
t.Fatalf("decodeMetadataHeader(%q, %q) = %q, %v, want %q, %v", test.kin, test.vin, v, err, test.vout, test.err)
}
}
}