Skip to content

Commit 1591994

Browse files
committed
Don't compress empty messages
Fixes #6831. This avoids compressing messages that are empty, since you can't compress zero bytes to anything smaller than zero bytes, and most compression algorithms output headers and trailers which means the resulting message will be non-zero bytes.
1 parent 1b05500 commit 1591994

File tree

2 files changed

+28
-10
lines changed

2 files changed

+28
-10
lines changed

rpc_util.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -640,14 +640,18 @@ func encode(c baseCodec, msg any) ([]byte, error) {
640640
return b, nil
641641
}
642642

643-
// compress returns the input bytes compressed by compressor or cp. If both
644-
// compressors are nil, returns nil.
643+
// compress returns the input bytes compressed by compressor or cp.
644+
// If both compressors are nil, or if the message has zero length, returns nil,
645+
// indicating no compression was done.
645646
//
646647
// TODO(dfawley): eliminate cp parameter by wrapping Compressor in an encoding.Compressor.
647648
func compress(in []byte, cp Compressor, compressor encoding.Compressor) ([]byte, error) {
648649
if compressor == nil && cp == nil {
649650
return nil, nil
650651
}
652+
if len(in) == 0 {
653+
return nil, nil
654+
}
651655
wrapErr := func(err error) error {
652656
return status.Errorf(codes.Internal, "grpc: error while compressing: %v", err.Error())
653657
}

test/compressor_test.go

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -290,19 +290,29 @@ func (s) TestSetSendCompressorSuccess(t *testing.T) {
290290
for _, tt := range []struct {
291291
name string
292292
desc string
293+
payload *testpb.Payload
293294
dialOpts []grpc.DialOption
294295
resCompressor string
295296
wantCompressInvokes int32
296297
}{
297298
{
298299
name: "identity_request_and_gzip_response",
299300
desc: "request is uncompressed and response is gzip compressed",
301+
payload: &testpb.Payload{Body: []byte("payload")},
300302
resCompressor: "gzip",
301303
wantCompressInvokes: 1,
302304
},
305+
{
306+
name: "identity_request_and_empty_response",
307+
desc: "request is uncompressed and response is gzip compressed",
308+
payload: nil,
309+
resCompressor: "gzip",
310+
wantCompressInvokes: 0,
311+
},
303312
{
304313
name: "gzip_request_and_identity_response",
305314
desc: "request is gzip compressed and response is uncompressed with identity",
315+
payload: &testpb.Payload{Body: []byte("payload")},
306316
resCompressor: "identity",
307317
dialOpts: []grpc.DialOption{
308318
// Use WithCompressor instead of UseCompressor to avoid counting
@@ -314,24 +324,26 @@ func (s) TestSetSendCompressorSuccess(t *testing.T) {
314324
} {
315325
t.Run(tt.name, func(t *testing.T) {
316326
t.Run("unary", func(t *testing.T) {
317-
testUnarySetSendCompressorSuccess(t, tt.resCompressor, tt.wantCompressInvokes, tt.dialOpts)
327+
testUnarySetSendCompressorSuccess(t, tt.payload, tt.resCompressor, tt.wantCompressInvokes, tt.dialOpts)
318328
})
319329

320330
t.Run("stream", func(t *testing.T) {
321-
testStreamSetSendCompressorSuccess(t, tt.resCompressor, tt.wantCompressInvokes, tt.dialOpts)
331+
testStreamSetSendCompressorSuccess(t, tt.payload, tt.resCompressor, tt.wantCompressInvokes, tt.dialOpts)
322332
})
323333
})
324334
}
325335
}
326336

327-
func testUnarySetSendCompressorSuccess(t *testing.T, resCompressor string, wantCompressInvokes int32, dialOpts []grpc.DialOption) {
337+
func testUnarySetSendCompressorSuccess(t *testing.T, payload *testpb.Payload, resCompressor string, wantCompressInvokes int32, dialOpts []grpc.DialOption) {
328338
wc := setupGzipWrapCompressor(t)
329339
ss := &stubserver.StubServer{
330-
EmptyCallF: func(ctx context.Context, in *testpb.Empty) (*testpb.Empty, error) {
340+
UnaryCallF: func(ctx context.Context, in *testpb.SimpleRequest) (*testpb.SimpleResponse, error) {
331341
if err := grpc.SetSendCompressor(ctx, resCompressor); err != nil {
332342
return nil, err
333343
}
334-
return &testpb.Empty{}, nil
344+
return &testpb.SimpleResponse{
345+
Payload: payload,
346+
}, nil
335347
},
336348
}
337349
if err := ss.Start(nil, dialOpts...); err != nil {
@@ -342,7 +354,7 @@ func testUnarySetSendCompressorSuccess(t *testing.T, resCompressor string, wantC
342354
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
343355
defer cancel()
344356

345-
if _, err := ss.Client.EmptyCall(ctx, &testpb.Empty{}); err != nil {
357+
if _, err := ss.Client.UnaryCall(ctx, &testpb.SimpleRequest{}); err != nil {
346358
t.Fatalf("Unexpected unary call error, got: %v, want: nil", err)
347359
}
348360

@@ -352,7 +364,7 @@ func testUnarySetSendCompressorSuccess(t *testing.T, resCompressor string, wantC
352364
}
353365
}
354366

355-
func testStreamSetSendCompressorSuccess(t *testing.T, resCompressor string, wantCompressInvokes int32, dialOpts []grpc.DialOption) {
367+
func testStreamSetSendCompressorSuccess(t *testing.T, payload *testpb.Payload, resCompressor string, wantCompressInvokes int32, dialOpts []grpc.DialOption) {
356368
wc := setupGzipWrapCompressor(t)
357369
ss := &stubserver.StubServer{
358370
FullDuplexCallF: func(stream testgrpc.TestService_FullDuplexCallServer) error {
@@ -364,7 +376,9 @@ func testStreamSetSendCompressorSuccess(t *testing.T, resCompressor string, want
364376
return err
365377
}
366378

367-
return stream.Send(&testpb.StreamingOutputCallResponse{})
379+
return stream.Send(&testpb.StreamingOutputCallResponse{
380+
Payload: payload,
381+
})
368382
},
369383
}
370384
if err := ss.Start(nil, dialOpts...); err != nil {

0 commit comments

Comments
 (0)