fix: Unconditionally quote string-encoded well-known types

PopulateOneField is called only for path params and query params, which are both unquoted.

util/genrest/goviewcreator.go

@@ -75,7 +75,7 @@ func NewView(model *gomodel.Model) (*goview.View, error) {
 
 			source.P("")
 			source.P("// %s translates REST requests/responses on the wire to internal proto messages for %s", handlerName, handler.GoMethod)
-			source.P("//    Generated for HTTP binding pattern: %q", handler.URIPattern)
+			source.P("//    Generated for HTTP binding pattern: %s %q", handler.HTTPMethod, handler.URIPattern)
 			source.P("func (backend *RESTBackend) %s(w http.ResponseWriter, r *http.Request) {", handlerName)
 			if handler.StreamingClient {
 				source.P(`  backend.Error(w, http.StatusNotImplemented, "client-streaming methods not implemented yet (request matched '%s': %%q)", r.URL)`, handler.URIPattern)

util/genrest/resttools/populatefield.go

@@ -24,43 +24,31 @@ import (
 	"google.golang.org/protobuf/reflect/protoreflect"
 )
 
-type valueTransform func(string) (string, error)
-
-func ensureQuotedValue(pre string) (string, error) {
-	hasStartQuote := strings.HasPrefix(pre, `"`)
-	hasEndQuote := strings.HasSuffix(pre, `"`)
-	if hasStartQuote != hasEndQuote {
-		return "", fmt.Errorf("unbalanced quotes in value %q", pre)
-	}
-	if hasStartQuote {
-		return pre, nil
-	}
-	return fmt.Sprintf(`"%s"`, pre), nil
-}
-
-var wellKnownTypes = map[string]valueTransform{
-	// testing various types in https://go.dev/play/p/rNhsXY364qQ
-
+// wellKnownTypes has a key for every well-known type message which JSON-encodes to an atomic symbol
+// (like a number or a string) as opposed to a structured object. The value for each key is true iff
+// the JSON encoding for the type is a string. We need both these data for well-known types so that
+// we can properly decode them in URL paths and query strings.
+var wellKnownTypes = map[string]bool{
 	// == The following are the only three common types used in this API ==
-	"google.protobuf.FieldMask": ensureQuotedValue,
-	"google.protobuf.Timestamp": ensureQuotedValue,
-	"google.protobuf.Duration":  ensureQuotedValue,
+	"google.protobuf.FieldMask": true,
+	"google.protobuf.Timestamp": true,
+	"google.protobuf.Duration":  true,
 	// == End utilized types ==
-	// TODO: When the following start being used int he Showcase API, add tests for their use as
-	// (unquoted) query params. These types are defined in
+	// TODO: When the following start being used in the Showcase API, add tests for each of
+	// these. These types are defined in
 	// https://github.com/protocolbuffers/protobuf/blob/main/src/google/protobuf/wrappers.proto
-	"google.protobuf.DoubleValue": nil,
-	"google.protobuf.FloatValue":  nil,
-	"google.protobuf.Int64Value":  ensureQuotedValue,
-	"google.protobuf.UInt64Value": ensureQuotedValue,
-	"google.protobuf.Int32Value":  nil,
-	"google.protobuf.UInt32Value": nil,
-	"google.protobuf.BoolValue":   nil,
-	"google.protobuf.StringValue": ensureQuotedValue,
-	"google.protobuf.BytesValue":  ensureQuotedValue,
+	"google.protobuf.DoubleValue": false,
+	"google.protobuf.FloatValue":  false,
+	"google.protobuf.Int64Value":  true,
+	"google.protobuf.UInt64Value": true,
+	"google.protobuf.Int32Value":  false,
+	"google.protobuf.UInt32Value": false,
+	"google.protobuf.BoolValue":   false,
+	"google.protobuf.StringValue": true,
+	"google.protobuf.BytesValue":  true,
 	// TODO: Determine if the following are even viable as query params.
-	"google.protobuf.Value":     nil,
-	"google.protobuf.ListValue": nil,
+	"google.protobuf.Value":     false,
+	"google.protobuf.ListValue": false,
 }
 
 // PopulateSingularFields sets the fields within protoMessage to the values provided in
@@ -153,6 +141,12 @@ func PopulateOneField(protoMessage proto.Message, fieldPath string, fieldValues
 		kind := fieldDescriptor.Kind()
 		switch kind {
 		case protoreflect.MessageKind:
+			// The only `MessageKind`s we accept in URL paths and query params are
+			// well-known types where the message as a whole encodes into a format
+			// similar to a regular terminal field. Normal messages conveyed via URL
+			// paths or query params must be non-repeated and are represented by listing
+			// their terminal primitive fields, which will trigger the other cases
+			// below instead of this one.
 			if pval, err := parseWellKnownType(message, fieldDescriptor, value); err != nil {
 				parseError = err
 			} else if pval != nil {
@@ -268,17 +262,13 @@ func parseWellKnownType(message protoreflect.Message, fieldDescriptor protorefle
 	}
 	fieldMsg := messageFieldTypes.Message(fieldDescriptor.Index())
 	fullName := string(fieldMsg.Descriptor().FullName())
-	transform, isWellKnown := wellKnownTypes[fullName]
+	stringEncoded, isWellKnown := wellKnownTypes[fullName]
 	if !isWellKnown {
 		return nil, nil
 	}
 
-	if transform != nil {
-		var err error
-		if value, err = transform(value); err != nil {
-			return nil, err
-		}
-
+	if stringEncoded {
+		value = fmt.Sprintf(`"%s"`, value)
 	}
 
 	msgValue := fieldMsg.New()

util/genrest/resttools/populatefield_test.go

@@ -21,7 +21,6 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	genprotopb "github.com/googleapis/gapic-showcase/server/genproto"
-	"google.golang.org/protobuf/encoding/protojson"
 	"google.golang.org/protobuf/encoding/prototext"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/reflect/protoreflect"
@@ -31,52 +30,6 @@ import (
 )
 
 func TestParseWellKnownType(t *testing.T) {
-	// TODO: Test the other well-known types as the Showcase API
-	// starts incorporating them.
-	for _, tst := range []struct {
-		name  string
-		msg   protoreflect.Message
-		field protoreflect.Name
-		want  proto.Message
-	}{
-		{
-			"google.protobuf.FieldMask",
-			(&genprotopb.UpdateUserRequest{}).ProtoReflect(),
-			"update_mask",
-			&fieldmaskpb.FieldMask{Paths: []string{"foo", "bar", "baz"}},
-		},
-		{
-			"google.protobuf.Timestamp",
-			(&genprotopb.User{}).ProtoReflect(),
-			"create_time",
-			timestamppb.Now(),
-		},
-		{
-			"google.protobuf.Duration",
-			(&genprotopb.Sequence_Response{}).ProtoReflect(),
-			"delay",
-			durationpb.New(5 * time.Second),
-		},
-	} {
-		data, _ := protojson.Marshal(tst.want)
-		value := string(data)
-		fd := tst.msg.Descriptor().Fields().ByName(tst.field)
-
-		gotp, err := parseWellKnownType(tst.msg, fd, value)
-		if err != nil {
-			t.Fatalf("parsing %q led to error %s", value, err)
-		}
-		if gotp == nil {
-			t.Fatalf("expected non-nil value from parsing: %s", value)
-		}
-		got := gotp.Message().Interface()
-		if diff := cmp.Diff(got, tst.want, cmp.Comparer(proto.Equal)); diff != "" {
-			t.Fatalf("%s: got(-),want(+):\n%s", "FieldMask", diff)
-		}
-	}
-}
-
-func TestParseWellKnownTypeUnquoted(t *testing.T) {
 	// TODO: Test the other well-known types as the Showcase API
 	// starts incorporating them.
 	for _, tst := range []struct {
@@ -90,7 +43,7 @@ func TestParseWellKnownTypeUnquoted(t *testing.T) {
 			"google.protobuf.FieldMask",
 			(&genprotopb.UpdateUserRequest{}).ProtoReflect(),
 			"update_mask",
-			`foo,bar,baz`,
+			"foo,bar,baz",
 			&fieldmaskpb.FieldMask{Paths: []string{"foo", "bar", "baz"}},
 		},