From 1425e2d86a81310bdefa95b352c3927d1da5f46e Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Mon, 12 May 2025 23:57:55 +0530 Subject: [PATCH 1/3] Avoid using negative deadline in test --- test/end2end_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/end2end_test.go b/test/end2end_test.go index fd5e29ea8c96..9e7239e28eb4 100644 --- a/test/end2end_test.go +++ b/test/end2end_test.go @@ -5342,7 +5342,7 @@ func testRPCTimeout(t *testing.T, e env) { ResponseSize: respSize, Payload: payload, } - for i := -1; i <= 10; i++ { + for i := 1; i <= 10; i++ { ctx, cancel := context.WithTimeout(context.Background(), time.Duration(i)*time.Millisecond) if _, err := tc.UnaryCall(ctx, req); status.Code(err) != codes.DeadlineExceeded { t.Fatalf("TestService/UnaryCallv(_, _) = _, %v; want , error code: %s", err, codes.DeadlineExceeded) From e1bcc17afb14eb1e092fee47b8afd0aa7fecea08 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Tue, 13 May 2025 10:38:46 +0530 Subject: [PATCH 2/3] Ensure positive timeout on client --- internal/transport/http2_client.go | 3 +++ test/end2end_test.go | 27 ++++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/internal/transport/http2_client.go b/internal/transport/http2_client.go index ed2d93af67e5..ef56592b944e 100644 --- a/internal/transport/http2_client.go +++ b/internal/transport/http2_client.go @@ -592,6 +592,9 @@ func (t *http2Client) createHeaderFields(ctx context.Context, callHdr *CallHdr) // Send out timeout regardless its value. The server can detect timeout context by itself. // TODO(mmukhi): Perhaps this field should be updated when actually writing out to the wire. timeout := time.Until(dl) + if timeout <= 0 { + return nil, status.Error(codes.DeadlineExceeded, context.DeadlineExceeded.Error()) + } headerFields = append(headerFields, hpack.HeaderField{Name: "grpc-timeout", Value: grpcutil.EncodeDuration(timeout)}) } for k, v := range authData { diff --git a/test/end2end_test.go b/test/end2end_test.go index 9e7239e28eb4..631da14a2141 100644 --- a/test/end2end_test.go +++ b/test/end2end_test.go @@ -5342,7 +5342,7 @@ func testRPCTimeout(t *testing.T, e env) { ResponseSize: respSize, Payload: payload, } - for i := 1; i <= 10; i++ { + for i := -1; i <= 10; i++ { ctx, cancel := context.WithTimeout(context.Background(), time.Duration(i)*time.Millisecond) if _, err := tc.UnaryCall(ctx, req); status.Code(err) != codes.DeadlineExceeded { t.Fatalf("TestService/UnaryCallv(_, _) = _, %v; want , error code: %s", err, codes.DeadlineExceeded) @@ -5351,6 +5351,31 @@ func testRPCTimeout(t *testing.T, e env) { } } +// Tests that the client doesn't send a negative timeout to the server. If the +// server receives a negative timeout, it would return an internal status. The +// client checks the context error before starting a stream, however the context +// may expire after this check and before the timeout is calculated. +func (s) TestNegativeRPCTimeout(t *testing.T) { + server := stubserver.StartTestService(t, nil) + defer server.Stop() + + if err := server.StartClient(); err != nil { + t.Fatal(err) + } + + // Try increasingly larger timeout values to trigger the condition when the + // context has expired while creating the grpc-timeout header. + for i := range 10 { + ctx, cancel := context.WithTimeout(context.Background(), time.Duration(i*100)*time.Nanosecond) + defer cancel() + + client := server.Client + if _, err := client.EmptyCall(ctx, &testpb.Empty{}); status.Code(err) != codes.DeadlineExceeded { + t.Fatalf("TestService/EmptyCall(_, _) = _, %v; want , error code: %s", err, codes.DeadlineExceeded) + } + } +} + func (s) TestDisabledIOBuffers(t *testing.T) { payload, err := newPayload(testpb.PayloadType_COMPRESSABLE, int32(60000)) if err != nil { From f8dc7c2e89fbeaae679ae7bdd25b49ba21d58d9d Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Wed, 14 May 2025 09:04:24 +0530 Subject: [PATCH 3/3] Improve fatalf log --- test/end2end_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/end2end_test.go b/test/end2end_test.go index 631da14a2141..75b27f4c224d 100644 --- a/test/end2end_test.go +++ b/test/end2end_test.go @@ -5360,7 +5360,7 @@ func (s) TestNegativeRPCTimeout(t *testing.T) { defer server.Stop() if err := server.StartClient(); err != nil { - t.Fatal(err) + t.Fatalf("Failed to create client: %v", err) } // Try increasingly larger timeout values to trigger the condition when the