Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add test of malformed gzip payload #3141

Merged
merged 5 commits into from Nov 5, 2019
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 44 additions & 0 deletions test/end2end_test.go
Expand Up @@ -24,6 +24,7 @@ package test
import (
"bufio"
"bytes"
"compress/gzip"
"context"
"crypto/tls"
"errors"
Expand Down Expand Up @@ -7525,3 +7526,46 @@ func (s) TestClientCancellationPropagatesUnary(t *testing.T) {
}
wg.Wait()
}

type badGzipCompressor struct{}

func (badGzipCompressor) Do(w io.Writer, p []byte) error {
buf := &bytes.Buffer{}
gzw := gzip.NewWriter(buf)
if _, err := gzw.Write(p); err != nil {
return err
}
err := gzw.Close()
menghanl marked this conversation as resolved.
Show resolved Hide resolved
bs := buf.Bytes()
bs[len(bs)-4] -= 1 // modify checksum (big endian) at end by 1 byte
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lint: should replace bs[len(bs)-4] -= 1 with bs[len(bs)-4]--

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

buf = bytes.NewBuffer(bs)
buf.WriteTo(w)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this same as w.Write(bs)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's simpler, thanks.

return err
}

func (badGzipCompressor) Type() string {
return "gzip"
}

func (s) TestGzipWithoutFooter(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is footer same as checksum?
And is this without footer or with invalid footer?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I was originally planning to remove it entirely, but that caused a different error to occur. (We read the last 4 bytes as the size before decoding and detect a message that would be too large upon decompression, so signal ResourceExhausted.) I changed the test to modify the checksum, which is in the previous 4 bytes of the footer.

ss := &stubServer{
unaryCall: func(ctx context.Context, _ *testpb.SimpleRequest) (*testpb.SimpleResponse, error) {
return &testpb.SimpleResponse{}, nil
},
}
if err := ss.Start(nil, grpc.WithCompressor(badGzipCompressor{})); err != nil {
t.Fatalf("Error starting endpoint server: %v", err)
}
defer ss.Stop()

ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()

p, err := newPayload(testpb.PayloadType_COMPRESSABLE, int32(1024))
if err != nil {
panic(err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

t.Fatalf

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}
if _, err := ss.client.UnaryCall(ctx, &testpb.SimpleRequest{Payload: p}); err == nil || status.Code(err) != codes.Internal {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we expect the error to contain gzip error message (gzip.ErrChecksum https://golang.org/pkg/compress/gzip/#pkg-variables)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

t.Errorf("ss.client.UnaryCall(_) = _, %v; want _, Code()=codes.Internal", err)
}
}