Skip to content

Commit

Permalink
Truncated S3 "get object" response is reported as error (#3795)
Browse files Browse the repository at this point in the history
* Add test to ensure S3 truncated response is an error

Signed-off-by: Marco Pracucci <marco@pracucci.com>

* Added missing copyright

Signed-off-by: Marco Pracucci <marco@pracucci.com>

* Upgraded Minio

Signed-off-by: Marco Pracucci <marco@pracucci.com>

* Upgraded Minio again

Signed-off-by: Marco Pracucci <marco@pracucci.com>
  • Loading branch information
pracucci authored Feb 24, 2021
1 parent 374576f commit f969003
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ We use _breaking :warning:_ to mark changes that are not backward compatible (re
### Fixed

- [#3773](https://github.com/thanos-io/thanos/pull/3773) Compact: Pad compaction planner size check
- [#3795](https://github.com/thanos-io/thanos/pull/3795) s3: A truncated "get object" response is reported as error.
- [#3814](https://github.com/thanos-io/thanos/pull/3814) Store: Decreased memory utilisation while fetching block's chunks.
- [#3815](https://github.com/thanos-io/thanos/pull/3815) Receive: Improve handling of empty time series from clients

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ require (
github.com/lightstep/lightstep-tracer-go v0.18.1
github.com/lovoo/gcloud-opentracing v0.3.0
github.com/miekg/dns v1.1.38
github.com/minio/minio-go/v7 v7.0.2
github.com/minio/minio-go/v7 v7.0.10
github.com/mozillazg/go-cos v0.13.0
github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f
github.com/ncw/swift v1.0.52
Expand Down
5 changes: 5 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,8 @@ github.com/minio/minio-go/v6 v6.0.44/go.mod h1:qD0lajrGW49lKZLtXKtCB4X/qkMf0a5tB
github.com/minio/minio-go/v6 v6.0.56/go.mod h1:KQMM+/44DSlSGSQWSfRrAZ12FVMmpWNuX37i2AX0jfI=
github.com/minio/minio-go/v7 v7.0.2 h1:P/7wFd4KrRBHVo7AKdcqO+9ReoS+XpMjfRFoE5quH0E=
github.com/minio/minio-go/v7 v7.0.2/go.mod h1:dJ80Mv2HeGkYLH1sqS/ksz07ON6csH3S6JUMSQ2zAns=
github.com/minio/minio-go/v7 v7.0.10 h1:1oUKe4EOPUEhw2qnPQaPsJ0lmVTYLFu03SiItauXs94=
github.com/minio/minio-go/v7 v7.0.10/go.mod h1:td4gW1ldOsj1PbSNS+WYK43j+P1XVhX/8W8awaYlBFo=
github.com/minio/sha256-simd v0.1.1 h1:5QHSlgo3nt5yKOJrC7W8w7X+NFl8cMPZm96iu8kKUJU=
github.com/minio/sha256-simd v0.1.1/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM=
github.com/mitchellh/cli v1.0.0/go.mod h1:hNIlj7HEI86fIcpObd7a0FcrxTWetlwJDGcceTlRvqc=
Expand Down Expand Up @@ -1111,6 +1113,8 @@ github.com/rogpeppe/go-internal v1.2.2/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFR
github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
github.com/rs/cors v1.6.0/go.mod h1:gFx+x8UowdsKA9AchylcLynDq+nNFfI8FkUZdN/jGCU=
github.com/rs/cors v1.7.0/go.mod h1:gFx+x8UowdsKA9AchylcLynDq+nNFfI8FkUZdN/jGCU=
github.com/rs/xid v1.2.1 h1:mhH9Nq+C1fY2l1XIpgxIiUOfNpRBYH1kKcr+qfKgjRc=
github.com/rs/xid v1.2.1/go.mod h1:+uKXf+4Djp6Md1KODXJxgGQPKngRmWyn10oCKFzNHOQ=
github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
github.com/ryanuber/columnize v2.1.0+incompatible/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
Expand Down Expand Up @@ -1332,6 +1336,7 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U
golang.org/x/crypto v0.0.0-20191112222119-e1110fd1c708/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20191202143827-86a70503ff7e/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20200709230013-948cd5f35899/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20200728195943-123391ffb6de/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0 h1:hb9wdF1z5waM+dSIICn1l0DkLVDT3hqhhQsDNUmHPRE=
golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
Expand Down
5 changes: 5 additions & 0 deletions pkg/objstore/gcs/gcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ func NewBucket(ctx context.Context, logger log.Logger, conf []byte, component st
if err := yaml.Unmarshal(conf, &gc); err != nil {
return nil, err
}

return NewBucketWithConfig(ctx, logger, gc, component)
}

func NewBucketWithConfig(ctx context.Context, logger log.Logger, gc Config, component string) (*Bucket, error) {
if gc.Bucket == "" {
return nil, errors.New("missing Google Cloud Storage bucket name for stored blocks")
}
Expand Down
47 changes: 47 additions & 0 deletions pkg/objstore/gcs/gcs_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright (c) The Thanos Authors.
// Licensed under the Apache License 2.0.

package gcs

import (
"context"
"io"
"io/ioutil"
"net/http"
"net/http/httptest"
"os"
"testing"

"github.com/go-kit/kit/log"

"github.com/thanos-io/thanos/pkg/testutil"
)

func TestBucket_Get_ShouldReturnErrorIfServerTruncateResponse(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Last-Modified", "Wed, 21 Oct 2015 07:28:00 GMT")
w.Header().Set("Content-Length", "100")

// Write less bytes than the content length.
_, err := w.Write([]byte("12345"))
testutil.Ok(t, err)
}))
defer srv.Close()

os.Setenv("STORAGE_EMULATOR_HOST", srv.Listener.Addr().String())

cfg := Config{
Bucket: "test-bucket",
ServiceAccount: "",
}

bkt, err := NewBucketWithConfig(context.Background(), log.NewNopLogger(), cfg, "test")
testutil.Ok(t, err)

reader, err := bkt.Get(context.Background(), "test")
testutil.Ok(t, err)

// We expect an error when reading back.
_, err = ioutil.ReadAll(reader)
testutil.Equals(t, io.ErrUnexpectedEOF, err)
}
34 changes: 34 additions & 0 deletions pkg/objstore/s3/s3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ package s3

import (
"context"
"io"
"io/ioutil"
"net/http"
"net/http/httptest"
"testing"
"time"

Expand Down Expand Up @@ -297,3 +301,33 @@ func TestBucket_getServerSideEncryption(t *testing.T) {
testutil.Ok(t, err)
testutil.Equals(t, encrypt.KMS, sse.Type())
}

func TestBucket_Get_ShouldReturnErrorIfServerTruncateResponse(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Last-Modified", "Wed, 21 Oct 2015 07:28:00 GMT")
w.Header().Set("Content-Length", "100")

// Write less bytes than the content length.
_, err := w.Write([]byte("12345"))
testutil.Ok(t, err)
}))
defer srv.Close()

cfg := DefaultConfig
cfg.Bucket = "test-bucket"
cfg.Endpoint = srv.Listener.Addr().String()
cfg.Insecure = true
cfg.Region = "test"
cfg.AccessKey = "test"
cfg.SecretKey = "test"

bkt, err := NewBucketWithConfig(log.NewNopLogger(), cfg, "test")
testutil.Ok(t, err)

reader, err := bkt.Get(context.Background(), "test")
testutil.Ok(t, err)

// We expect an error when reading back.
_, err = ioutil.ReadAll(reader)
testutil.Equals(t, io.ErrUnexpectedEOF, err)
}

0 comments on commit f969003

Please sign in to comment.