diff options
author | Paul Okstad <pokstad@gitlab.com> | 2020-01-14 21:50:51 +0300 |
---|---|---|
committer | John Cai <jcai@gitlab.com> | 2020-01-14 21:50:51 +0300 |
commit | d7d6ecbcdcd6776a96fd87cfcfeec6335a4a471c (patch) | |
tree | 6d5d47ae5ec59f096fc890422146410e69a82890 | |
parent | 6c3d0035f975270149cff3a1200887b02d8d5293 (diff) |
Fix middleware to stop panicking from bad requests
-rw-r--r-- | changelogs/unreleased/po-fix-invalid-request-cache-panic.yml | 5 | ||||
-rw-r--r-- | internal/middleware/cache/cache.go | 3 | ||||
-rw-r--r-- | internal/service/repository/archive_test.go | 11 | ||||
-rw-r--r-- | internal/service/repository/testhelper_test.go | 13 |
4 files changed, 29 insertions, 3 deletions
diff --git a/changelogs/unreleased/po-fix-invalid-request-cache-panic.yml b/changelogs/unreleased/po-fix-invalid-request-cache-panic.yml new file mode 100644 index 000000000..bb48399df --- /dev/null +++ b/changelogs/unreleased/po-fix-invalid-request-cache-panic.yml @@ -0,0 +1,5 @@ +--- +title: Fix middleware to stop panicking from bad requests +merge_request: 1747 +author: +type: fixed diff --git a/internal/middleware/cache/cache.go b/internal/middleware/cache/cache.go index 0a3415ed5..79d8066e0 100644 --- a/internal/middleware/cache/cache.go +++ b/internal/middleware/cache/cache.go @@ -161,11 +161,12 @@ func invalidateCache(ci Invalidator, mInfo protoregistry.MethodInfo, handler grp le.Lock() defer le.Unlock() - le.LeaseEnder, err = ci.StartLease(target) + l, err := ci.StartLease(target) if err != nil { errLogger(err) return } + le.LeaseEnder = l } return wrappedHandler, peekerCallback diff --git a/internal/service/repository/archive_test.go b/internal/service/repository/archive_test.go index 16d9e278b..a5afbc1f1 100644 --- a/internal/service/repository/archive_test.go +++ b/internal/service/repository/archive_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/streamio" @@ -199,11 +200,21 @@ func TestGetArchiveFailure(t *testing.T) { path: []byte("../../foo"), code: codes.InvalidArgument, }, + { + desc: "repo missing fields", + repo: &gitalypb.Repository{StorageName: "default"}, + prefix: "qwert", + commitID: "sadf", + format: gitalypb.GetArchiveRequest_TAR, + path: []byte("Here is a string...."), + code: codes.InvalidArgument, + }, } for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { ctx, cancel := testhelper.Context() + ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.CacheInvalidator) defer cancel() req := &gitalypb.GetArchiveRequest{ diff --git a/internal/service/repository/testhelper_test.go b/internal/service/repository/testhelper_test.go index 8c9d73d7a..37c5e6d89 100644 --- a/internal/service/repository/testhelper_test.go +++ b/internal/service/repository/testhelper_test.go @@ -12,7 +12,10 @@ import ( "github.com/stretchr/testify/assert" gitalyauth "gitlab.com/gitlab-org/gitaly/auth" "gitlab.com/gitlab-org/gitaly/client" + dcache "gitlab.com/gitlab-org/gitaly/internal/cache" "gitlab.com/gitlab-org/gitaly/internal/config" + mcache "gitlab.com/gitlab-org/gitaly/internal/middleware/cache" + "gitlab.com/gitlab-org/gitaly/internal/praefect/protoregistry" "gitlab.com/gitlab-org/gitaly/internal/rubyserver" "gitlab.com/gitlab-org/gitaly/internal/server/auth" "gitlab.com/gitlab-org/gitaly/internal/testhelper" @@ -64,8 +67,14 @@ func newSecureRepoClient(t *testing.T, serverSocketPath string, pool *x509.CertP var NewSecureRepoClient = newSecureRepoClient func runRepoServer(t *testing.T) (*grpc.Server, string) { - streamInt := []grpc.StreamServerInterceptor{auth.StreamServerInterceptor(config.Config.Auth)} - unaryInt := []grpc.UnaryServerInterceptor{auth.UnaryServerInterceptor(config.Config.Auth)} + streamInt := []grpc.StreamServerInterceptor{ + auth.StreamServerInterceptor(config.Config.Auth), + mcache.StreamInvalidator(dcache.LeaseKeyer{}, protoregistry.GitalyProtoPreregistered), + } + unaryInt := []grpc.UnaryServerInterceptor{ + auth.UnaryServerInterceptor(config.Config.Auth), + mcache.UnaryInvalidator(dcache.LeaseKeyer{}, protoregistry.GitalyProtoPreregistered), + } server := testhelper.NewTestGrpcServer(t, streamInt, unaryInt) serverSocketPath := testhelper.GetTemporaryGitalySocketFileName() |