diff --git a/internal/gitaly/service/hook/pre_receive.go b/internal/gitaly/service/hook/pre_receive.go index 4062546dc2acc2d07b1543170a3a6240e58a453b..6beeb4da461c12550582f30e38e82d3b06aca2b2 100644 --- a/internal/gitaly/service/hook/pre_receive.go +++ b/internal/gitaly/service/hook/pre_receive.go @@ -7,10 +7,14 @@ import ( "os/exec" "sync" + "github.com/golang/protobuf/ptypes/wrappers" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" + "gitlab.com/gitlab-org/gitaly/v16/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v16/internal/structerr" "gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v16/streamio" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/wrapperspb" ) func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer) error { @@ -61,11 +65,56 @@ func validatePreReceiveHookRequest(ctx context.Context, locator storage.Locator, return locator.ValidateRepository(ctx, in.GetRepository()) } +type preReceiveHookErrorSender struct { + errorCode int32 + bytes *wrappers.BytesValue + send func(*wrappers.BytesValue, int32) error +} + +func (p *preReceiveHookErrorSender) Reset() { + if p.bytes != nil { + p.bytes.Value = p.bytes.Value[:0] + return + } + + p.bytes = &wrappers.BytesValue{} +} + +func (p *preReceiveHookErrorSender) Append(m proto.Message) { + p.bytes.Value = append(p.bytes.Value, m.(*wrappers.BytesValue).Value...) +} + +func (p *preReceiveHookErrorSender) Send() error { + return p.send(p.bytes, p.errorCode) +} + func preReceiveHookResponse(stream gitalypb.HookService_PreReceiveHookServer, code int32, stderr string) error { - if err := stream.Send(&gitalypb.PreReceiveHookResponse{ - ExitStatus: &gitalypb.ExitStatus{Value: code}, - Stderr: []byte(stderr), - }); err != nil { + if stderr == "" { + if err := stream.Send(&gitalypb.PreReceiveHookResponse{ + ExitStatus: &gitalypb.ExitStatus{Value: code}, + Stderr: []byte(""), + }); err != nil { + return structerr.NewInternal("sending response: %w", err) + } + + return nil + } + + chunker := chunk.New(&preReceiveHookErrorSender{ + errorCode: code, + send: func(bytes *wrappers.BytesValue, errorCode int32) error { + return stream.Send(&gitalypb.PreReceiveHookResponse{ + ExitStatus: &gitalypb.ExitStatus{Value: errorCode}, + Stderr: []byte(bytes.Value), + }) + }, + }) + + if err := chunker.Send(wrapperspb.Bytes([]byte(stderr))); err != nil { + return structerr.NewInternal("sending response: %w", err) + } + + if err := chunker.Flush(); err != nil { return structerr.NewInternal("sending response: %w", err) } diff --git a/internal/gitaly/service/hook/pre_receive_test.go b/internal/gitaly/service/hook/pre_receive_test.go index b0d50c11a8b0104fe428ed3275e0e0fe10c59ec3..6dc0751063fb7c29eba6d18cc0d75e63e3aff8b7 100644 --- a/internal/gitaly/service/hook/pre_receive_test.go +++ b/internal/gitaly/service/hook/pre_receive_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/http/httptest" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -350,52 +351,71 @@ func TestPreReceiveHook_CustomHookErrors(t *testing.T) { ctx := testhelper.Context(t) repo, repoPath := gittest.CreateRepository(t, ctx, cfg) - customHookReturnCode := int32(128) - customHookReturnMsg := "custom hook error" + testCases := []struct { + desc string + returnCode int32 + returnMsg string + }{ + { + desc: "normal custom hook error", + returnCode: 128, + returnMsg: "custom hook error", + }, + { + desc: "large error message", + returnCode: 1, + returnMsg: strings.Repeat("a", (4*1024*1024)+1), + }, + } - //nolint:gitaly-linters - gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte(fmt.Sprintf(`#!/usr/bin/env bash + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + //nolint:gitaly-linters + gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte(fmt.Sprintf(`#!/usr/bin/env bash echo '%s' 1>&2 exit %d -`, customHookReturnMsg, customHookReturnCode))) +`, tc.returnMsg, tc.returnCode))) - client, conn := newHooksClient(t, cfg.SocketPath) - defer conn.Close() + client, conn := newHooksClient(t, cfg.SocketPath) + defer conn.Close() - hooksPayload, err := gitcmd.NewHooksPayload( - ctx, - cfg, - repo, - gittest.DefaultObjectHash, - nil, - &gitcmd.UserDetails{ - UserID: "key-123", - Username: "username", - Protocol: "web", - }, - gitcmd.PreReceiveHook, - featureflag.FromContext(ctx), - storage.ExtractTransactionID(ctx), - ).Env() - require.NoError(t, err) + hooksPayload, err := gitcmd.NewHooksPayload( + ctx, + cfg, + repo, + gittest.DefaultObjectHash, + nil, + &gitcmd.UserDetails{ + UserID: "key-123", + Username: "username", + Protocol: "web", + }, + gitcmd.PreReceiveHook, + featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), + ).Env() + require.NoError(t, err) - stream, err := client.PreReceiveHook(ctx) - require.NoError(t, err) - require.NoError(t, stream.Send(&gitalypb.PreReceiveHookRequest{ - Repository: repo, - EnvironmentVariables: []string{ - hooksPayload, - }, - })) - require.NoError(t, stream.Send(&gitalypb.PreReceiveHookRequest{ - Stdin: []byte("changes\n"), - })) - require.NoError(t, stream.CloseSend()) + stream, err := client.PreReceiveHook(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&gitalypb.PreReceiveHookRequest{ + Repository: repo, + EnvironmentVariables: []string{ + hooksPayload, + }, + })) + require.NoError(t, stream.Send(&gitalypb.PreReceiveHookRequest{ + Stdin: []byte("changes\n"), + })) + require.NoError(t, stream.CloseSend()) - _, stderr, status := receivePreReceive(t, stream, &bytes.Buffer{}) + _, stderr, status := receivePreReceive(t, stream, &bytes.Buffer{}) + + require.Equal(t, tc.returnCode, status) + assert.Equal(t, tc.returnMsg, text.ChompBytes(stderr), "hook stderr") + }) + } - require.Equal(t, customHookReturnCode, status) - assert.Equal(t, customHookReturnMsg, text.ChompBytes(stderr), "hook stderr") } func TestPreReceiveHook_Primary(t *testing.T) {