Merge branch 'jsonpb-unknown-fields' into 'master'

Allow unknown fields in jsonpb gitaly-proto messages

Closes gitlab-ce#57589

See merge request gitlab-org/gitlab-workhorse!367

internal/git/diff.go

@@ -4,8 +4,6 @@ import (
 	"fmt"
 	"net/http"
 
-	"github.com/golang/protobuf/jsonpb"
-
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/gitaly"
@@ -29,7 +27,7 @@ func (d *diff) Inject(w http.ResponseWriter, r *http.Request, sendData string) {
 	}
 
 	request := &gitalypb.RawDiffRequest{}
-	if err := jsonpb.UnmarshalString(params.RawDiffRequest, request); err != nil {
+	if err := gitaly.UnmarshalJSON(params.RawDiffRequest, request); err != nil {
 		helper.Fail500(w, r, fmt.Errorf("diff.RawDiff: %v", err))
 		return
 	}

internal/git/format-patch.go

@@ -4,8 +4,6 @@ import (
 	"fmt"
 	"net/http"
 
-	"github.com/golang/protobuf/jsonpb"
-
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/gitaly"
@@ -29,7 +27,7 @@ func (p *patch) Inject(w http.ResponseWriter, r *http.Request, sendData string)
 	}
 
 	request := &gitalypb.RawPatchRequest{}
-	if err := jsonpb.UnmarshalString(params.RawPatchRequest, request); err != nil {
+	if err := gitaly.UnmarshalJSON(params.RawPatchRequest, request); err != nil {
 		helper.Fail500(w, r, fmt.Errorf("diff.RawPatch: %v", err))
 		return
 	}

internal/git/snapshot.go

@@ -5,8 +5,6 @@ import (
 	"io"
 	"net/http"
 
-	"github.com/golang/protobuf/jsonpb"
-
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/gitaly"
@@ -36,7 +34,7 @@ func (s *snapshot) Inject(w http.ResponseWriter, r *http.Request, sendData strin
 	}
 
 	request := &gitalypb.GetSnapshotRequest{}
-	if err := jsonpb.UnmarshalString(params.GetSnapshotRequest, request); err != nil {
+	if err := gitaly.UnmarshalJSON(params.GetSnapshotRequest, request); err != nil {
 		helper.Fail500(w, r, fmt.Errorf("SendSnapshot: unmarshal GetSnapshotRequest: %v", err))
 		return
 	}

internal/gitaly/gitaly.go

 package gitaly
 
 import (
+	"strings"
 	"sync"
 
+	"github.com/golang/protobuf/jsonpb"
+	"github.com/golang/protobuf/proto"
 	grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
 	grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus"
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
@@ -24,9 +27,12 @@ type connectionsCache struct {
 	connections map[Server]*grpc.ClientConn
 }
 
-var cache = connectionsCache{
-	connections: make(map[Server]*grpc.ClientConn),
-}
+var (
+	jsonUnMarshaler = jsonpb.Unmarshaler{AllowUnknownFields: true}
+	cache           = connectionsCache{
+		connections: make(map[Server]*grpc.ClientConn),
+	}
+)
 
 func NewSmartHTTPClient(server Server) (*SmartHTTPClient, error) {
 	conn, err := getOrCreateConnection(server)
@@ -131,3 +137,7 @@ func newConnection(server Server) (*grpc.ClientConn, error) {
 
 	return gitalyclient.Dial(server.Address, connOpts)
 }
+
+func UnmarshalJSON(s string, msg proto.Message) error {
+	return jsonUnMarshaler.Unmarshal(strings.NewReader(s), msg)
+}

internal/gitaly/unmarshal_test.go

+package gitaly
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
+)
+
+func TestUnmarshalJSON(t *testing.T) {
+	testCases := []struct {
+		desc string
+		in   string
+		out  gitalypb.Repository
+	}{
+		{
+			desc: "basic example",
+			in:   `{"relative_path":"foo/bar.git"}`,
+			out:  gitalypb.Repository{RelativePath: "foo/bar.git"},
+		},
+		{
+			desc: "unknown field",
+			in:   `{"relative_path":"foo/bar.git","unknown_field":12345}`,
+			out:  gitalypb.Repository{RelativePath: "foo/bar.git"},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			result := gitalypb.Repository{}
+			require.NoError(t, UnmarshalJSON(tc.in, &result))
+			require.Equal(t, tc.out, result)
+		})
+	}
+}