rpc_util: fix bug of getting empty response when compress enabled and maxReceiveMessageSize be maxInt64 #7468
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7468 +/- ##
==========================================
- Coverage 82.07% 81.74% -0.34%
==========================================
Files 360 360
Lines 27533 27521 -12
==========================================
- Hits 22599 22497 -102
- Misses 3759 3810 +51
- Partials 1175 1214 +39
|
|
@infovivek2020 please revert the changes in Few more things about convention
|
|
@infovivek2020 can you please fix the PR title and description as mentioned above |
infovivek2020
changed the title
infovivek2020
changed the title
|
@purnesh42H Please have a look PR i have added unit test case for decompress function |
|
@infovivek2020 please rebase from latest and undo the unrelated changes |
infovivek2020
force-pushed
the
4552-compression-issue-empty-response
branch
3 times, most recently
from
a2a1757 to
d53d3f5
Compare
|
@purnesh42H please review this PR rebase done |
purnesh42H
changed the title
purnesh42H
changed the title
| @@ -809,25 +810,50 @@ func decompress(compressor encoding.Compressor, d []byte, maxReceiveMessageSize | |||
| }); ok { | |||
| if size := sizer.DecompressedSize(d); size >= 0 { | |||
| if size > maxReceiveMessageSize { | |||
| return nil, size, nil | |||
| return nil, size, errors.New("message size exceeds maximum allowed") | |||
There was a problem hiding this comment.
suggestion: received message size is more than allowed maxReceiveMessageSize
| @@ -266,3 +267,133 @@ func BenchmarkGZIPCompressor512KiB(b *testing.B) { | |||
| func BenchmarkGZIPCompressor1MiB(b *testing.B) { | |||
| bmCompressor(b, 1024*1024, NewGZIPCompressor()) | |||
| } | |||
| func TestCheckReceiveMessageOverflow(t *testing.T) { | |||
There was a problem hiding this comment.
one line separator before TestCheckReceiveMessageOverflow
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| err := checkReceiveMessageOverflow(tt.readBytes, tt.maxReceiveMessageSize, tt.dcReader) | ||
| if (err != nil) != (tt.wantErr != nil) { |
There was a problem hiding this comment.
if tt.wantErr != nil && err == nil {
t.Errorf("checkReceiveMessageOverflow(): got nil error, want an error")
}
if err != nil && err.Error() != tt.wantErr.Error() {
t.Errorf("checkReceiveMessageOverflow(): got err=%v, want err=%v", err, tt.wantErr)
}
```
|
|
||
| if (err != nil) != tt.wantErr { | ||
| t.Errorf("decompress() error = %v, wantErr %v", err, tt.wantErr) | ||
| return |
There was a problem hiding this comment.
no need of return since you are raising the error before
| } | ||
| } | ||
|
|
||
| type testCompressor struct { |
There was a problem hiding this comment.
can we move the helper structs and functions to the top before test functions?
| return | ||
| } | ||
| if !bytes.Equal(output, tt.wantOutput) { | ||
| t.Errorf("decompress() got = %v, want %v", output, tt.wantOutput) |
There was a problem hiding this comment.
missing colon (:) after decompress
| t.Errorf("decompress() got = %v, want %v", output, tt.wantOutput) | ||
| } | ||
| if size != tt.wantSize { | ||
| t.Errorf("decompress() size = %d, want %d", size, tt.wantSize) |
There was a problem hiding this comment.
colon (:) and got after decompress()
| } | ||
|
|
||
| // TestDecompress tests the decompress function. | ||
| func TestDecompress(t *testing.T) { |
There was a problem hiding this comment.
Try to cover the testcase of dcReader overflow when reading extra byte and remove the separate tests for CheckOverflow
infovivek2020
force-pushed
the
4552-compression-issue-empty-response
branch
from
8c7d5c6 to
259244c
Compare
|
@infovivek2020 please fix the merge commits and reopen the PR or submit a new PR from latest main branch |
Fixes #4552
RELEASE NOTES: fix bug of getting empty response when compress enabled and maxReceiveMessageSize be maxInt64