Skip to content

Commit e1bcc17

Browse files
committed
Ensure positive timeout on client
1 parent 1425e2d commit e1bcc17

File tree

2 files changed

+29
-1
lines changed

2 files changed

+29
-1
lines changed

internal/transport/http2_client.go

+3
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,9 @@ func (t *http2Client) createHeaderFields(ctx context.Context, callHdr *CallHdr)
592592
// Send out timeout regardless its value. The server can detect timeout context by itself.
593593
// TODO(mmukhi): Perhaps this field should be updated when actually writing out to the wire.
594594
timeout := time.Until(dl)
595+
if timeout <= 0 {
596+
return nil, status.Error(codes.DeadlineExceeded, context.DeadlineExceeded.Error())
597+
}
595598
headerFields = append(headerFields, hpack.HeaderField{Name: "grpc-timeout", Value: grpcutil.EncodeDuration(timeout)})
596599
}
597600
for k, v := range authData {

test/end2end_test.go

+26-1
Original file line numberDiff line numberDiff line change
@@ -5342,7 +5342,7 @@ func testRPCTimeout(t *testing.T, e env) {
53425342
ResponseSize: respSize,
53435343
Payload: payload,
53445344
}
5345-
for i := 1; i <= 10; i++ {
5345+
for i := -1; i <= 10; i++ {
53465346
ctx, cancel := context.WithTimeout(context.Background(), time.Duration(i)*time.Millisecond)
53475347
if _, err := tc.UnaryCall(ctx, req); status.Code(err) != codes.DeadlineExceeded {
53485348
t.Fatalf("TestService/UnaryCallv(_, _) = _, %v; want <nil>, error code: %s", err, codes.DeadlineExceeded)
@@ -5351,6 +5351,31 @@ func testRPCTimeout(t *testing.T, e env) {
53515351
}
53525352
}
53535353

5354+
// Tests that the client doesn't send a negative timeout to the server. If the
5355+
// server receives a negative timeout, it would return an internal status. The
5356+
// client checks the context error before starting a stream, however the context
5357+
// may expire after this check and before the timeout is calculated.
5358+
func (s) TestNegativeRPCTimeout(t *testing.T) {
5359+
server := stubserver.StartTestService(t, nil)
5360+
defer server.Stop()
5361+
5362+
if err := server.StartClient(); err != nil {
5363+
t.Fatal(err)
5364+
}
5365+
5366+
// Try increasingly larger timeout values to trigger the condition when the
5367+
// context has expired while creating the grpc-timeout header.
5368+
for i := range 10 {
5369+
ctx, cancel := context.WithTimeout(context.Background(), time.Duration(i*100)*time.Nanosecond)
5370+
defer cancel()
5371+
5372+
client := server.Client
5373+
if _, err := client.EmptyCall(ctx, &testpb.Empty{}); status.Code(err) != codes.DeadlineExceeded {
5374+
t.Fatalf("TestService/EmptyCall(_, _) = _, %v; want <nil>, error code: %s", err, codes.DeadlineExceeded)
5375+
}
5376+
}
5377+
}
5378+
53545379
func (s) TestDisabledIOBuffers(t *testing.T) {
53555380
payload, err := newPayload(testpb.PayloadType_COMPRESSABLE, int32(60000))
53565381
if err != nil {

0 commit comments

Comments
 (0)