-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
client: use "localhost:port" as authority if target is ":port" #4017
Changes from 4 commits
b82a372
16bcbdc
34b5d5c
e41f1ae
0ca3dba
1132a36
080bc24
f700819
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 ( | |
"fmt" | ||
"net" | ||
"os" | ||
"strings" | ||
"testing" | ||
"time" | ||
|
||
|
@@ -33,44 +34,48 @@ import ( | |
testpb "google.golang.org/grpc/test/grpc_testing" | ||
) | ||
|
||
func authorityChecker(expectedAuthority *string) func(context.Context, *testpb.Empty) (*testpb.Empty, error) { | ||
return func(ctx context.Context, in *testpb.Empty) (*testpb.Empty, error) { | ||
md, ok := metadata.FromIncomingContext(ctx) | ||
if !ok { | ||
return nil, status.Error(codes.InvalidArgument, "failed to parse metadata") | ||
} | ||
auths, ok := md[":authority"] | ||
if !ok { | ||
return nil, status.Error(codes.InvalidArgument, "no authority header") | ||
} | ||
if len(auths) != 1 { | ||
return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("no authority header, auths = %v", auths)) | ||
} | ||
if auths[0] != *expectedAuthority { | ||
return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("invalid authority header %v, expected %v", auths[0], *expectedAuthority)) | ||
} | ||
return &testpb.Empty{}, nil | ||
} | ||
} | ||
|
||
func runUnixTest(t *testing.T, address, target, expectedAuthority string, dialer func(context.Context, string) (net.Conn, error)) { | ||
if err := os.RemoveAll(address); err != nil { | ||
t.Fatalf("Error removing socket file %v: %v\n", address, err) | ||
} | ||
us := &stubServer{ | ||
emptyCall: func(ctx context.Context, in *testpb.Empty) (*testpb.Empty, error) { | ||
md, ok := metadata.FromIncomingContext(ctx) | ||
if !ok { | ||
return nil, status.Error(codes.InvalidArgument, "failed to parse metadata") | ||
} | ||
auths, ok := md[":authority"] | ||
if !ok { | ||
return nil, status.Error(codes.InvalidArgument, "no authority header") | ||
} | ||
if len(auths) < 1 { | ||
return nil, status.Error(codes.InvalidArgument, "no authority header") | ||
} | ||
if auths[0] != expectedAuthority { | ||
return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("invalid authority header %v, expected %v", auths[0], expectedAuthority)) | ||
} | ||
return &testpb.Empty{}, nil | ||
}, | ||
network: "unix", | ||
address: address, | ||
target: target, | ||
ss := &stubServer{ | ||
emptyCall: authorityChecker(&expectedAuthority), | ||
network: "unix", | ||
address: address, | ||
target: target, | ||
} | ||
opts := []grpc.DialOption{} | ||
if dialer != nil { | ||
opts = append(opts, grpc.WithContextDialer(dialer)) | ||
} | ||
if err := us.Start(nil, opts...); err != nil { | ||
if err := ss.Start(nil, opts...); err != nil { | ||
t.Fatalf("Error starting endpoint server: %v", err) | ||
return | ||
} | ||
defer us.Stop() | ||
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) | ||
defer ss.Stop() | ||
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) | ||
defer cancel() | ||
_, err := us.client.EmptyCall(ctx, &testpb.Empty{}) | ||
_, err := ss.client.EmptyCall(ctx, &testpb.Empty{}) | ||
if err != nil { | ||
t.Errorf("us.client.EmptyCall(_, _) = _, %v; want _, nil", err) | ||
} | ||
|
@@ -147,3 +152,36 @@ func (s) TestUnixCustomDialer(t *testing.T) { | |
}) | ||
} | ||
} | ||
|
||
func (s) TestColonPortAuthority(t *testing.T) { | ||
expectedAuthority := "" | ||
ss := &stubServer{ | ||
emptyCall: authorityChecker(&expectedAuthority), | ||
network: "tcp", | ||
} | ||
if err := ss.Start(nil); err != nil { | ||
t.Fatalf("Error starting endpoint server: %v", err) | ||
return | ||
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. return-after-fatal is unncessary. 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. Removed return |
||
} | ||
defer ss.Stop() | ||
s := strings.Split(ss.address, ":") | ||
if len(s) != 2 { | ||
t.Fatalf("No port in address: %v", ss.address) | ||
return | ||
} | ||
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. Use 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. Using net package for this now. |
||
expectedAuthority = "localhost:" + s[1] | ||
// ss.Start dials, but not the ":[port]" target that is being tested here. | ||
// Dial again, with ":[port]" as the target. | ||
cc, err := grpc.Dial(":"+s[1], grpc.WithInsecure()) | ||
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.
|
||
if err != nil { | ||
t.Fatalf("grpc.Dial(%q) = %v", ss.target, err) | ||
return | ||
} | ||
defer cc.Close() | ||
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) | ||
defer cancel() | ||
_, err = testpb.NewTestServiceClient(cc).EmptyCall(ctx, &testpb.Empty{}) | ||
if err != nil { | ||
t.Errorf("us.client.EmptyCall(_, _) = _, %v; want _, nil", err) | ||
} | ||
} |
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'm not sure about passing a pointer here without synchronization. It may fail the race test.
I'd do this instead:
Or use a channel to pass the expected authority to the handler, but that's probably more trouble than it's worth.
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.
Made this change.