From ac7cc4f799f2d4fa19d41d11f496113f252020b2 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Wed, 18 Dec 2024 00:50:01 +0530 Subject: [PATCH 1/5] Add a test for decompression exceeding max receive message size --- test/compressor_test.go | 61 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/test/compressor_test.go b/test/compressor_test.go index 0495a06f0968..eb7ab7f1837e 100644 --- a/test/compressor_test.go +++ b/test/compressor_test.go @@ -785,3 +785,64 @@ func (s) TestGzipBadChecksum(t *testing.T) { t.Errorf("ss.Client.UnaryCall(_) = _, %v\n\twant: _, status(codes.Internal, contains %q)", err, gzip.ErrChecksum) } } + +// fakeCompressor returns a messages of a configured size, irrespective of the +// input. +type fakeCompressor struct { + decompressedMessageSize int +} + +func (f *fakeCompressor) Compress(w io.Writer) (io.WriteCloser, error) { + return nopWriteCloser{w}, nil +} + +func (f *fakeCompressor) Decompress(io.Reader) (io.Reader, error) { + return bytes.NewReader(make([]byte, f.decompressedMessageSize)), nil +} + +func (f *fakeCompressor) Name() string { + return "fake-compressor" +} + +type nopWriteCloser struct { + io.Writer +} + +func (nopWriteCloser) Close() error { + return nil +} + +// TestDecompressionExceedsMaxMessageSize uses a fake compressor that produces +// messages of size 100 bytes on decompression. A server is started with the +// max receive message size restricted to 99 bytes. The test verifies that the +// client receives a ResourceExhausted response from the server. +func (s) TestDecompressionExceedsMaxMessageSize(t *testing.T) { + ss := &stubserver.StubServer{ + UnaryCallF: func(context.Context, *testpb.SimpleRequest) (*testpb.SimpleResponse, error) { + return &testpb.SimpleResponse{}, nil + }, + } + messageLen := 100 + encoding.RegisterCompressor(&fakeCompressor{decompressedMessageSize: messageLen}) + if err := ss.Start([]grpc.ServerOption{grpc.MaxRecvMsgSize(messageLen - 1)}); err != nil { + t.Fatalf("Error starting endpoint server: %v", err) + } + defer ss.Stop() + + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + + p, err := newPayload(testpb.PayloadType_COMPRESSABLE, int32(50)) + if err != nil { + t.Fatalf("Unexpected error from newPayload: %v", err) + } + req := &testpb.SimpleRequest{Payload: p} + _, err = ss.Client.UnaryCall(ctx, req, grpc.UseCompressor("fake-compressor")) + if err == nil { + t.Errorf("Client.UnaryCall(%+v) = nil, want %v", req, codes.ResourceExhausted) + } + + if got, want := status.Code(err), codes.ResourceExhausted; got != want { + t.Errorf("Client.UnaryCall(%+v) returned stats %v, want %v", req, got, want) + } +} From 9ef12af4b3213bc0e78976755a66f45ac9248420 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Wed, 18 Dec 2024 01:05:02 +0530 Subject: [PATCH 2/5] Use existing compressor name --- test/compressor_test.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/test/compressor_test.go b/test/compressor_test.go index eb7ab7f1837e..444b1314b6f4 100644 --- a/test/compressor_test.go +++ b/test/compressor_test.go @@ -801,7 +801,9 @@ func (f *fakeCompressor) Decompress(io.Reader) (io.Reader, error) { } func (f *fakeCompressor) Name() string { - return "fake-compressor" + // Use the name of an existing compressor to avoid interactions with other + // tests since compressors can't be un-registered. + return "gzip" } type nopWriteCloser struct { @@ -817,13 +819,17 @@ func (nopWriteCloser) Close() error { // max receive message size restricted to 99 bytes. The test verifies that the // client receives a ResourceExhausted response from the server. func (s) TestDecompressionExceedsMaxMessageSize(t *testing.T) { + oldC := encoding.GetCompressor("gzip") + defer func() { + encoding.RegisterCompressor(oldC) + }() + messageLen := 100 + encoding.RegisterCompressor(&fakeCompressor{decompressedMessageSize: messageLen}) ss := &stubserver.StubServer{ UnaryCallF: func(context.Context, *testpb.SimpleRequest) (*testpb.SimpleResponse, error) { return &testpb.SimpleResponse{}, nil }, } - messageLen := 100 - encoding.RegisterCompressor(&fakeCompressor{decompressedMessageSize: messageLen}) if err := ss.Start([]grpc.ServerOption{grpc.MaxRecvMsgSize(messageLen - 1)}); err != nil { t.Fatalf("Error starting endpoint server: %v", err) } @@ -837,7 +843,7 @@ func (s) TestDecompressionExceedsMaxMessageSize(t *testing.T) { t.Fatalf("Unexpected error from newPayload: %v", err) } req := &testpb.SimpleRequest{Payload: p} - _, err = ss.Client.UnaryCall(ctx, req, grpc.UseCompressor("fake-compressor")) + _, err = ss.Client.UnaryCall(ctx, req, grpc.UseCompressor("gzip")) if err == nil { t.Errorf("Client.UnaryCall(%+v) = nil, want %v", req, codes.ResourceExhausted) } From a4e00051a650b2faef694717066295454252700e Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Wed, 18 Dec 2024 20:26:44 +0530 Subject: [PATCH 3/5] Address review --- test/compressor_test.go | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/test/compressor_test.go b/test/compressor_test.go index 444b1314b6f4..0a629bcecdb5 100644 --- a/test/compressor_test.go +++ b/test/compressor_test.go @@ -823,7 +823,7 @@ func (s) TestDecompressionExceedsMaxMessageSize(t *testing.T) { defer func() { encoding.RegisterCompressor(oldC) }() - messageLen := 100 + const messageLen = 100 encoding.RegisterCompressor(&fakeCompressor{decompressedMessageSize: messageLen}) ss := &stubserver.StubServer{ UnaryCallF: func(context.Context, *testpb.SimpleRequest) (*testpb.SimpleResponse, error) { @@ -838,17 +838,9 @@ func (s) TestDecompressionExceedsMaxMessageSize(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer cancel() - p, err := newPayload(testpb.PayloadType_COMPRESSABLE, int32(50)) - if err != nil { - t.Fatalf("Unexpected error from newPayload: %v", err) - } - req := &testpb.SimpleRequest{Payload: p} - _, err = ss.Client.UnaryCall(ctx, req, grpc.UseCompressor("gzip")) - if err == nil { - t.Errorf("Client.UnaryCall(%+v) = nil, want %v", req, codes.ResourceExhausted) - } - + req := &testpb.SimpleRequest{Payload: &testpb.Payload{}} + _, err := ss.Client.UnaryCall(ctx, req, grpc.UseCompressor("gzip")) if got, want := status.Code(err), codes.ResourceExhausted; got != want { - t.Errorf("Client.UnaryCall(%+v) returned stats %v, want %v", req, got, want) + t.Errorf("Client.UnaryCall(%+v) returned status %v, want %v", req, got, want) } } From 63320aaad24e2763762f8e136fcb69be56f36000 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Mon, 23 Dec 2024 09:33:13 +0530 Subject: [PATCH 4/5] Don't use gzip name --- test/compressor_test.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/test/compressor_test.go b/test/compressor_test.go index 0a629bcecdb5..298ae38ec1b5 100644 --- a/test/compressor_test.go +++ b/test/compressor_test.go @@ -801,9 +801,7 @@ func (f *fakeCompressor) Decompress(io.Reader) (io.Reader, error) { } func (f *fakeCompressor) Name() string { - // Use the name of an existing compressor to avoid interactions with other - // tests since compressors can't be un-registered. - return "gzip" + return "fake-gzip" } type nopWriteCloser struct { @@ -819,10 +817,6 @@ func (nopWriteCloser) Close() error { // max receive message size restricted to 99 bytes. The test verifies that the // client receives a ResourceExhausted response from the server. func (s) TestDecompressionExceedsMaxMessageSize(t *testing.T) { - oldC := encoding.GetCompressor("gzip") - defer func() { - encoding.RegisterCompressor(oldC) - }() const messageLen = 100 encoding.RegisterCompressor(&fakeCompressor{decompressedMessageSize: messageLen}) ss := &stubserver.StubServer{ @@ -839,7 +833,7 @@ func (s) TestDecompressionExceedsMaxMessageSize(t *testing.T) { defer cancel() req := &testpb.SimpleRequest{Payload: &testpb.Payload{}} - _, err := ss.Client.UnaryCall(ctx, req, grpc.UseCompressor("gzip")) + _, err := ss.Client.UnaryCall(ctx, req, grpc.UseCompressor("fake-gzip")) if got, want := status.Code(err), codes.ResourceExhausted; got != want { t.Errorf("Client.UnaryCall(%+v) returned status %v, want %v", req, got, want) } From 1bf3afa60ef870a4dfc56c0ca361360510ede943 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Mon, 23 Dec 2024 09:39:50 +0530 Subject: [PATCH 5/5] Revert "Don't use gzip name" This reverts commit 63320aaad24e2763762f8e136fcb69be56f36000. --- test/compressor_test.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/compressor_test.go b/test/compressor_test.go index 298ae38ec1b5..0a629bcecdb5 100644 --- a/test/compressor_test.go +++ b/test/compressor_test.go @@ -801,7 +801,9 @@ func (f *fakeCompressor) Decompress(io.Reader) (io.Reader, error) { } func (f *fakeCompressor) Name() string { - return "fake-gzip" + // Use the name of an existing compressor to avoid interactions with other + // tests since compressors can't be un-registered. + return "gzip" } type nopWriteCloser struct { @@ -817,6 +819,10 @@ func (nopWriteCloser) Close() error { // max receive message size restricted to 99 bytes. The test verifies that the // client receives a ResourceExhausted response from the server. func (s) TestDecompressionExceedsMaxMessageSize(t *testing.T) { + oldC := encoding.GetCompressor("gzip") + defer func() { + encoding.RegisterCompressor(oldC) + }() const messageLen = 100 encoding.RegisterCompressor(&fakeCompressor{decompressedMessageSize: messageLen}) ss := &stubserver.StubServer{ @@ -833,7 +839,7 @@ func (s) TestDecompressionExceedsMaxMessageSize(t *testing.T) { defer cancel() req := &testpb.SimpleRequest{Payload: &testpb.Payload{}} - _, err := ss.Client.UnaryCall(ctx, req, grpc.UseCompressor("fake-gzip")) + _, err := ss.Client.UnaryCall(ctx, req, grpc.UseCompressor("gzip")) if got, want := status.Code(err), codes.ResourceExhausted; got != want { t.Errorf("Client.UnaryCall(%+v) returned status %v, want %v", req, got, want) }