Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

metadata: validate metadata keys and values #4886

Merged
merged 26 commits into from Feb 23, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions call.go
Expand Up @@ -20,13 +20,20 @@ package grpc

import (
"context"

"google.golang.org/grpc/metadata"
)

// Invoke sends the RPC request on the wire and returns after response is
// received. This is typically called by generated code.
//
// All errors returned by Invoke are compatible with the status package.
func (cc *ClientConn) Invoke(ctx context.Context, method string, args, reply interface{}, opts ...CallOption) error {
if md, _ ,ok := metadata.FromOutgoingContextRaw(ctx); ok {
Patrick0308 marked this conversation as resolved.
Show resolved Hide resolved
if err := md.Validate(); err != nil {
return err
}
}
// allow interceptor to see all applicable call options, which means those
// configured as defaults from dial option as well as per-call options
opts = combine(cc.dopts.callOptions, opts)
Expand Down
39 changes: 39 additions & 0 deletions metadata/metadata.go
Expand Up @@ -23,6 +23,7 @@ package metadata // import "google.golang.org/grpc/metadata"

import (
"context"
"errors"
"fmt"
"strings"
)
Expand Down Expand Up @@ -82,6 +83,44 @@ func Pairs(kv ...string) MD {
return md
}

// Validate returns error if md is not valid
// There are check items:
// - header names contain one or more characters from this set [0-9 a-z _ - .]
// - if the header-name ends with a "-bin" suffix, the header-value could contain an arbitrary octet sequence. So no real validation required here.
// - if header-name does not end with a "-bin" suffix, header-value should only contain one or more characters from the set ( %x20-%x7E ) which includes space and printable ASCII.
func (md MD) Validate() error {
Patrick0308 marked this conversation as resolved.
Show resolved Hide resolved
for k, vals := range md {
// check key, for i that saving a conversion if not using for range
for i := 0; i< len(k); i++ {
r := k[i]
if !(r >= 'a' && r <= 'z') && !(r >= '0' && r <= '9') && r != '.' && r != '-' && r != '_' {
return errors.New("header key is not 0-9a-z-_.")
}
}
if strings.HasSuffix(k, "-bin") {
continue
}
// check value
for _, val := range vals {
if hasNotPrintable(val) {
return errors.New("header val has not printable ASCII")
}
}
}
return nil
}

// hasNotPrintable return true if msg has character not in %x20-%x7E
func hasNotPrintable(msg string) bool {
// for i that saving a conversion if not using for range
for i := 0; i < len(msg); i++ {
if msg[i] < 0x20 || msg[i] > 0x7E {
return true
}
}
return false
}

// Len returns the number of items in md.
func (md MD) Len() int {
return len(md)
Expand Down
26 changes: 26 additions & 0 deletions metadata/metadata_test.go
Expand Up @@ -20,6 +20,7 @@ package metadata

import (
"context"
"errors"
Patrick0308 marked this conversation as resolved.
Show resolved Hide resolved
"reflect"
"strconv"
"testing"
Expand Down Expand Up @@ -262,6 +263,31 @@ func (s) TestAppendToOutgoingContext_FromKVSlice(t *testing.T) {
}
}

func (s) TestValidate(t *testing.T) {
Patrick0308 marked this conversation as resolved.
Show resolved Hide resolved
for _, test := range []struct {
md MD
want error
}{
{
md: Pairs(string(rune(0x19)), "testVal"),
want: errors.New("header key is not 0-9a-z-_."),
},
{
md: Pairs("test", string(rune(0x19))),
want: errors.New("header val has not printable ASCII"),
},
{
md: Pairs("test-bin", string(rune(0x19))),
want: nil,
},
} {
err := test.md.Validate()
if !reflect.DeepEqual(err, test.want) {
t.Errorf("validating metadata which is %v got err :%v, want err :%v", test.md, err, test.want)
}
}
}

// Old/slow approach to adding metadata to context
func Benchmark_AddingMetadata_ContextManipulationApproach(b *testing.B) {
// TODO: Add in N=1-100 tests once Go1.6 support is removed.
Expand Down
14 changes: 13 additions & 1 deletion stream.go
Expand Up @@ -1446,11 +1446,20 @@ func (ss *serverStream) SetHeader(md metadata.MD) error {
if md.Len() == 0 {
return nil
}
err := md.Validate()
if err != nil {
return err
}
return ss.s.SetHeader(md)
}

func (ss *serverStream) SendHeader(md metadata.MD) error {
err := ss.t.WriteHeader(ss.s, md)
err := md.Validate()
if err != nil {
return err
}

err = ss.t.WriteHeader(ss.s, md)
if ss.binlog != nil && !ss.serverHeaderBinlogged {
h, _ := ss.s.Header()
ss.binlog.Log(&binarylog.ServerHeader{
Expand All @@ -1465,6 +1474,9 @@ func (ss *serverStream) SetTrailer(md metadata.MD) {
if md.Len() == 0 {
return
}
if md.Validate() != nil {
return
Patrick0308 marked this conversation as resolved.
Show resolved Hide resolved
Patrick0308 marked this conversation as resolved.
Show resolved Hide resolved
}
ss.s.SetTrailer(md)
}

Expand Down