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
grpc: better RFC 3986 compliant target parsing #4817
Changes from 19 commits
fb12d07
cee1b44
d19cd9b
0d0c1a5
c98608a
fb4b9c0
2750421
c522792
4f56e2e
ae490d0
99fdc43
f578ae5
1fa26dc
7936989
499f603
c5c4383
5cff707
c36957f
8546581
78fcb25
4fb05c3
c6057e5
f230c53
5f96946
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ import ( | |
"errors" | ||
"fmt" | ||
"math" | ||
"net/url" | ||
"reflect" | ||
"strings" | ||
"sync" | ||
|
@@ -37,7 +38,6 @@ import ( | |
"google.golang.org/grpc/internal/backoff" | ||
"google.golang.org/grpc/internal/channelz" | ||
"google.golang.org/grpc/internal/grpcsync" | ||
"google.golang.org/grpc/internal/grpcutil" | ||
iresolver "google.golang.org/grpc/internal/resolver" | ||
"google.golang.org/grpc/internal/transport" | ||
"google.golang.org/grpc/keepalive" | ||
|
@@ -248,38 +248,12 @@ func DialContext(ctx context.Context, target string, opts ...DialOption) (conn * | |
} | ||
|
||
// Determine the resolver to use. | ||
cc.parsedTarget = grpcutil.ParseTarget(cc.target, cc.dopts.copts.Dialer != nil) | ||
channelz.Infof(logger, cc.channelzID, "parsed scheme: %q", cc.parsedTarget.Scheme) | ||
resolverBuilder := cc.getResolver(cc.parsedTarget.Scheme) | ||
if resolverBuilder == nil { | ||
// If resolver builder is still nil, the parsed target's scheme is | ||
// not registered. Fallback to default resolver and set Endpoint to | ||
// the original target. | ||
channelz.Infof(logger, cc.channelzID, "scheme %q not registered, fallback to default scheme", cc.parsedTarget.Scheme) | ||
cc.parsedTarget = resolver.Target{ | ||
Scheme: resolver.GetDefaultScheme(), | ||
Endpoint: target, | ||
} | ||
resolverBuilder = cc.getResolver(cc.parsedTarget.Scheme) | ||
if resolverBuilder == nil { | ||
return nil, fmt.Errorf("could not get resolver for default scheme: %q", cc.parsedTarget.Scheme) | ||
} | ||
} | ||
|
||
creds := cc.dopts.copts.TransportCredentials | ||
if creds != nil && creds.Info().ServerName != "" { | ||
cc.authority = creds.Info().ServerName | ||
} else if cc.dopts.insecure && cc.dopts.authority != "" { | ||
cc.authority = cc.dopts.authority | ||
} else if strings.HasPrefix(cc.target, "unix:") || strings.HasPrefix(cc.target, "unix-abstract:") { | ||
cc.authority = "localhost" | ||
} else if strings.HasPrefix(cc.parsedTarget.Endpoint, ":") { | ||
cc.authority = "localhost" + cc.parsedTarget.Endpoint | ||
} else { | ||
// Use endpoint from "scheme://authority/endpoint" as the default | ||
// authority for ClientConn. | ||
cc.authority = cc.parsedTarget.Endpoint | ||
resolverBuilder, err := cc.parseTargetAndFindResolver() | ||
if err != nil { | ||
return nil, err | ||
} | ||
cc.determineAuthority() | ||
channelz.Infof(logger, cc.channelzID, "Channel authority set to %q", cc.authority) | ||
|
||
if cc.dopts.scChan != nil && !scSet { | ||
// Blocking wait for the initial service config. | ||
|
@@ -902,10 +876,7 @@ func (ac *addrConn) tryUpdateAddrs(addrs []resolver.Address) bool { | |
// ac.state is Ready, try to find the connected address. | ||
var curAddrFound bool | ||
for _, a := range addrs { | ||
// a.ServerName takes precedent over ClientConn authority, if present. | ||
if a.ServerName == "" { | ||
a.ServerName = ac.cc.authority | ||
} | ||
a.ServerName = ac.cc.getServerName(a) | ||
if reflect.DeepEqual(ac.curAddr, a) { | ||
curAddrFound = true | ||
break | ||
|
@@ -919,6 +890,26 @@ func (ac *addrConn) tryUpdateAddrs(addrs []resolver.Address) bool { | |
return curAddrFound | ||
} | ||
|
||
// getServerName determines the serverName to be used in the connection | ||
// handshake. The default value for the serverName is the authority on the | ||
// ClientConn, which either comes from the user's dial target or through an | ||
// authority override specified using the WithAuthority dial option. Name | ||
// resolvers can specify a per-address override for the serverName through the | ||
// resolver.Address.ServerName field which is used only if the WithAuthority | ||
// dial option was not used. The rationale is that per-address authority | ||
// overrides specified by the name resolver can represent a security risk, while | ||
// an override specified by the user is more dependable since they probably know | ||
// what they are doing. | ||
func (cc *ClientConn) getServerName(addr resolver.Address) string { | ||
if cc.dopts.authority != "" { | ||
return cc.dopts.authority | ||
} | ||
if addr.ServerName != "" { | ||
return addr.ServerName | ||
} | ||
return cc.authority | ||
} | ||
|
||
func getMethodConfig(sc *ServiceConfig, method string) MethodConfig { | ||
if sc == nil { | ||
return MethodConfig{} | ||
|
@@ -1275,11 +1266,7 @@ func (ac *addrConn) createTransport(addr resolver.Address, copts transport.Conne | |
prefaceReceived := grpcsync.NewEvent() | ||
connClosed := grpcsync.NewEvent() | ||
|
||
// addr.ServerName takes precedent over ClientConn authority, if present. | ||
if addr.ServerName == "" { | ||
addr.ServerName = ac.cc.authority | ||
} | ||
|
||
addr.ServerName = ac.cc.getServerName(addr) | ||
hctx, hcancel := context.WithCancel(ac.ctx) | ||
hcStarted := false // protected by ac.mu | ||
|
||
|
@@ -1621,3 +1608,119 @@ func (cc *ClientConn) connectionError() error { | |
defer cc.lceMu.Unlock() | ||
return cc.lastConnectionError | ||
} | ||
|
||
func (cc *ClientConn) parseTargetAndFindResolver() (resolver.Builder, error) { | ||
channelz.Infof(logger, cc.channelzID, "original dial target is: %q", cc.target) | ||
|
||
var rb resolver.Builder | ||
parsedTarget, err := parseTarget(cc.target) | ||
if err != nil { | ||
channelz.Infof(logger, cc.channelzID, "dial target %q parse failed: %v", cc.target, err) | ||
} else { | ||
channelz.Infof(logger, cc.channelzID, "parsed dial target is: %+v", parsedTarget) | ||
rb = cc.getResolver(parsedTarget.Scheme) | ||
if rb != nil { | ||
cc.parsedTarget = parsedTarget | ||
return rb, nil | ||
} | ||
} | ||
|
||
// We are here because the user's dial target did not contain a scheme or | ||
// specified an unregistered scheme. We should fallback to the default | ||
// scheme, except when a custom dialer is specified in which case, we should | ||
// always use passthrough scheme. | ||
defScheme := resolver.GetDefaultScheme() | ||
if cc.dopts.copts.Dialer != nil { | ||
defScheme = "passthrough" | ||
} | ||
channelz.Infof(logger, cc.channelzID, "fallback to scheme %q", defScheme) | ||
canonicalTarget := defScheme + ":///" + cc.target | ||
|
||
parsedTarget, err = parseTarget(canonicalTarget) | ||
if err != nil { | ||
channelz.Infof(logger, cc.channelzID, "dial target %q parse failed: %v", canonicalTarget, err) | ||
return nil, err | ||
} | ||
channelz.Infof(logger, cc.channelzID, "parsed dial target is: %+v", parsedTarget) | ||
rb = cc.getResolver(parsedTarget.Scheme) | ||
if rb == nil { | ||
return nil, fmt.Errorf("could not get resolver for default scheme: %q", parsedTarget.Scheme) | ||
} | ||
cc.parsedTarget = parsedTarget | ||
return rb, nil | ||
} | ||
|
||
// parseTarget uses RFC 3986 semantics to parse the given target into a | ||
// resolver.Target struct containing scheme, authority and endpoint. Query | ||
// params are stripped from the endpoint. | ||
func parseTarget(target string) (resolver.Target, error) { | ||
u, err := url.Parse(target) | ||
if err != nil { | ||
return resolver.Target{}, err | ||
} | ||
// For targets of the form "[scheme]://[authority]/endpoint, the endpoint | ||
// value returned from url.Parse() contains a leading "/". Although this is | ||
// in accordance with RFC 3986, we do not want to break existing resolver | ||
// implementations which expect the endpoint without the leading "/". So, we | ||
// end up stripping the leading "/" here. But this will result in an | ||
// incorrect parsing for something like "unix:///path/to/socket". Since we | ||
// own the "unix" resolver, we can workaround in the unix resolver by using | ||
// the `ParsedURL` field instead of the `Endpoint` field. | ||
endpoint := u.Path | ||
if endpoint == "" { | ||
endpoint = u.Opaque | ||
} | ||
endpoint = strings.TrimPrefix(endpoint, "/") | ||
return resolver.Target{ | ||
Scheme: u.Scheme, | ||
Authority: u.Host, | ||
Endpoint: endpoint, | ||
ParsedURL: *u, | ||
}, nil | ||
} | ||
|
||
// Determine channel authority. The order of precedence is as follows: | ||
// - user specified authority override using `WithAuthority` dial option | ||
// - creds' notion of server name for the authentication handshake | ||
// - endpoint from dial target of the form "scheme://[authority]/endpoint" | ||
func (cc *ClientConn) determineAuthority() { | ||
// Historically, we had two options for users to specify the serverName or | ||
// authority for a channel. One was through the transport credentials | ||
// (either in its constructor, or through the OverrideServerName() method). | ||
// The other option (for cases where WithInsecure() dial option was used) | ||
// was to use the WithAuthority() dial option. | ||
// | ||
// A few things have changed since: | ||
// - `insecure` package with an implementation of the `TransportCredentials` | ||
// interface for the insecure case | ||
// - WithAuthority() dial option support for secure credentials | ||
authorityFromCreds := "" | ||
if creds := cc.dopts.copts.TransportCredentials; creds != nil && creds.Info().ServerName != "" { | ||
authorityFromCreds = creds.Info().ServerName | ||
} | ||
authorityFromDialOption := "" | ||
if cc.dopts.authority != "" { | ||
authorityFromDialOption = cc.dopts.authority | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Simplify: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
if (authorityFromCreds != "" && authorityFromDialOption != "") && authorityFromCreds != authorityFromDialOption { | ||
channelz.Warningf(logger, cc.channelzID, "ClientConn's authority from transport creds %q and dial option %q don't match. Will use the former.", authorityFromCreds, authorityFromDialOption) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should return an error here. The Creds+WithAuthority combination wasn't even allowed in the past so now that we're allowing it I'd rather fail if they are both specified and not consistent. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
|
||
// TODO: Define an optional interface on the resolver builder to return the | ||
// channel authority given the user's dial target. Once we have this, we can | ||
// get rid of the special cases for unix and `:port`. | ||
switch { | ||
case authorityFromDialOption != "": | ||
cc.authority = authorityFromDialOption | ||
case authorityFromCreds != "": | ||
cc.authority = authorityFromCreds | ||
case strings.HasPrefix(cc.target, "unix:") || strings.HasPrefix(cc.target, "unix-abstract:"): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
cc.authority = "localhost" | ||
case strings.HasPrefix(cc.parsedTarget.Endpoint, ":"): | ||
cc.authority = "localhost" + cc.parsedTarget.Endpoint | ||
default: | ||
// Use endpoint from "scheme://authority/endpoint" as the default | ||
// authority for ClientConn. | ||
cc.authority = cc.parsedTarget.Endpoint | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,9 @@ import ( | |
"testing" | ||
"time" | ||
|
||
"github.com/google/go-cmp/cmp" | ||
"github.com/google/go-cmp/cmp/cmpopts" | ||
|
||
"google.golang.org/grpc/resolver" | ||
) | ||
|
||
|
@@ -45,7 +48,7 @@ func (s) TestParsedTarget_Success_WithoutCustomDialer(t *testing.T) { | |
{target: "a/b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a/b"}}, | ||
{target: "a//b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a//b"}}, | ||
{target: "google.com", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "google.com"}}, | ||
{target: "google.com/?a=b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "google.com/?a=b"}}, | ||
{target: "google.com/?a=b", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "google.com/"}}, | ||
{target: "/unix/socket/address", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/unix/socket/address"}}, | ||
|
||
// An unregistered scheme is specified. | ||
|
@@ -60,20 +63,22 @@ func (s) TestParsedTarget_Success_WithoutCustomDialer(t *testing.T) { | |
// A registered scheme is specified. | ||
{target: "dns:///google.com", wantParsed: resolver.Target{Scheme: "dns", Authority: "", Endpoint: "google.com"}}, | ||
{target: "dns://a.server.com/google.com", wantParsed: resolver.Target{Scheme: "dns", Authority: "a.server.com", Endpoint: "google.com"}}, | ||
{target: "dns://a.server.com/google.com/?a=b", wantParsed: resolver.Target{Scheme: "dns", Authority: "a.server.com", Endpoint: "google.com/?a=b"}}, | ||
{target: "unix:///a/b/c", wantParsed: resolver.Target{Scheme: "unix", Authority: "", Endpoint: "/a/b/c"}}, | ||
{target: "dns://a.server.com/google.com/?a=b", wantParsed: resolver.Target{Scheme: "dns", Authority: "a.server.com", Endpoint: "google.com/"}}, | ||
{target: "unix:///a/b/c", wantParsed: resolver.Target{Scheme: "unix", Authority: "", Endpoint: "a/b/c"}}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's validate the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since I marked the older fields as deprecated, I added the |
||
{target: "unix-abstract:a/b/c", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "a/b/c"}}, | ||
{target: "unix-abstract:a b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "a b"}}, | ||
{target: "unix-abstract:a:b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "a:b"}}, | ||
{target: "unix-abstract:a-b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "a-b"}}, | ||
{target: "unix-abstract:/ a///://::!@#$%^&*()b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "/ a///://::!@#$%^&*()b"}}, | ||
{target: "unix-abstract:/ a///://::!@#$%25^&*()b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: " a///://::!@"}}, | ||
{target: "unix-abstract:passthrough:abc", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "passthrough:abc"}}, | ||
{target: "unix-abstract:unix:///abc", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "unix:///abc"}}, | ||
{target: "unix-abstract:///a/b/c", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "/a/b/c"}}, | ||
{target: "unix-abstract:///", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "/"}}, | ||
{target: "unix-abstract://authority", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "//authority"}}, | ||
{target: "unix://domain", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "unix://domain"}}, | ||
{target: "unix-abstract:///a/b/c", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "a/b/c"}}, | ||
{target: "unix-abstract:///", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: ""}}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you delete the other two test cases? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do not support |
||
{target: "passthrough:///unix:///a/b/c", wantParsed: resolver.Target{Scheme: "passthrough", Authority: "", Endpoint: "unix:///a/b/c"}}, | ||
|
||
// Cases for `scheme:absolute-path`. | ||
{target: "dns:/a/b/c", wantParsed: resolver.Target{Scheme: "dns", Authority: "", Endpoint: "a/b/c"}}, | ||
{target: "unregistered:/a/b/c", wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "unregistered:/a/b/c"}}, | ||
} | ||
|
||
for _, test := range tests { | ||
|
@@ -84,8 +89,8 @@ func (s) TestParsedTarget_Success_WithoutCustomDialer(t *testing.T) { | |
} | ||
defer cc.Close() | ||
|
||
if gotParsed := cc.parsedTarget; gotParsed != test.wantParsed { | ||
t.Errorf("cc.parsedTarget = %+v, want %+v", gotParsed, test.wantParsed) | ||
if !cmp.Equal(cc.parsedTarget, test.wantParsed, cmpopts.IgnoreFields(resolver.Target{}, "ParsedURL")) { | ||
t.Errorf("cc.parsedTarget for dial target %q = %+v, want %+v", test.target, cc.parsedTarget, test.wantParsed) | ||
} | ||
}) | ||
} | ||
|
@@ -94,7 +99,9 @@ func (s) TestParsedTarget_Success_WithoutCustomDialer(t *testing.T) { | |
func (s) TestParsedTarget_Failure_WithoutCustomDialer(t *testing.T) { | ||
targets := []string{ | ||
"unix://a/b/c", | ||
"unix://authority", | ||
"unix-abstract://authority/a/b/c", | ||
"unix-abstract://authority", | ||
} | ||
|
||
for _, target := range targets { | ||
|
@@ -118,17 +125,17 @@ func (s) TestParsedTarget_WithCustomDialer(t *testing.T) { | |
// different behaviors with a custom dialer. | ||
{ | ||
target: "unix:a/b/c", | ||
wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "unix:a/b/c"}, | ||
wantParsed: resolver.Target{Scheme: "unix", Authority: "", Endpoint: "a/b/c"}, | ||
wantDialerAddress: "unix:a/b/c", | ||
}, | ||
{ | ||
target: "unix:/a/b/c", | ||
wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "unix:/a/b/c"}, | ||
wantDialerAddress: "unix:/a/b/c", | ||
wantParsed: resolver.Target{Scheme: "unix", Authority: "", Endpoint: "a/b/c"}, | ||
wantDialerAddress: "unix:///a/b/c", | ||
}, | ||
{ | ||
target: "unix:///a/b/c", | ||
wantParsed: resolver.Target{Scheme: "unix", Authority: "", Endpoint: "/a/b/c"}, | ||
wantParsed: resolver.Target{Scheme: "unix", Authority: "", Endpoint: "a/b/c"}, | ||
wantDialerAddress: "unix:///a/b/c", | ||
}, | ||
{ | ||
|
@@ -151,6 +158,16 @@ func (s) TestParsedTarget_WithCustomDialer(t *testing.T) { | |
wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "://authority/127.0.0.1:50051"}, | ||
wantDialerAddress: "://authority/127.0.0.1:50051", | ||
}, | ||
{ | ||
target: "/unix/socket/address", | ||
wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/unix/socket/address"}, | ||
wantDialerAddress: "/unix/socket/address", | ||
}, | ||
{ | ||
target: "passthrough://a.server.com/google.com", | ||
wantParsed: resolver.Target{Scheme: "passthrough", Authority: "a.server.com", Endpoint: "google.com"}, | ||
wantDialerAddress: "google.com", | ||
}, | ||
} | ||
|
||
for _, test := range tests { | ||
|
@@ -175,8 +192,8 @@ func (s) TestParsedTarget_WithCustomDialer(t *testing.T) { | |
case <-time.After(time.Second): | ||
t.Fatal("timeout when waiting for custom dialer to be invoked") | ||
} | ||
if gotParsed := cc.parsedTarget; gotParsed != test.wantParsed { | ||
t.Errorf("cc.parsedTarget for dial target %q = %+v, want %+v", test.target, gotParsed, test.wantParsed) | ||
if !cmp.Equal(cc.parsedTarget, test.wantParsed, cmpopts.IgnoreFields(resolver.Target{}, "ParsedURL")) { | ||
t.Errorf("cc.parsedTarget for dial target %q = %+v, want %+v", test.target, cc.parsedTarget, test.wantParsed) | ||
} | ||
}) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -140,6 +140,13 @@ type TransportCredentials interface { | |
// Additionally, ClientHandshakeInfo data will be available via the context | ||
// passed to this call. | ||
// | ||
// The second argument to this method is the ClientConn's authority, usually | ||
// derived from the user's dial target. Implementations should use this as | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, ClientConn's authority? But what if the ServerName on the address overrides it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reworded it a little. |
||
// the server name during the authentication handshake. Implementations may | ||
// choose to ignore this value and use a value acquired through other means, | ||
// in which case they must make sure that the value is acquired through | ||
// secure means and that a possible attacker cannot tamper with the value. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Holy cow, we should never, ever recommend (or even mention) ignoring this field. Implementations MUST honor this value, period. Otherwise the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
// | ||
// If the returned net.Conn is closed, it MUST close the net.Conn provided. | ||
ClientHandshake(context.Context, string, net.Conn) (net.Conn, AuthInfo, error) | ||
// ServerHandshake does the authentication handshake for servers. It returns | ||
|
@@ -153,9 +160,13 @@ type TransportCredentials interface { | |
Info() ProtocolInfo | ||
// Clone makes a copy of this TransportCredentials. | ||
Clone() TransportCredentials | ||
// OverrideServerName overrides the server name used to verify the hostname on the returned certificates from the server. | ||
// gRPC internals also use it to override the virtual hosting name if it is set. | ||
// It must be called before dialing. Currently, this is only used by grpclb. | ||
// OverrideServerName specifies the value used for the following: | ||
// - verifying the hostname on the returned certificates | ||
// - as SNI in the client's handshake to support virtual hosting | ||
// - as the value for `:authority` header at stream creation time | ||
// | ||
// Deprecated: use grpc.WithAuthority instead. Will be supported | ||
// throughout 1.x. | ||
OverrideServerName(string) error | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this may be cleaner if you passed dopts as a parameter, returned the authority, and didn't make this a method. I'd rather see
cc.authority := determineAuthority(cc.dopts)
thancc.determineAuthority()
since in the former case I can reason about what might and might not be happening, whereas in the latter case I need to inspectcc.determineAuthority
to determine what all it might be doing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.