jhump / protoreflect

Reflection (Rich Descriptors) for Go Protocol Buffers
Apache License 2.0
1.35k stars 172 forks source link

If there are messages nested in the proto file, the numbers will be recognised as strings #599

Closed Big-big-orange closed 6 months ago

Big-big-orange commented 7 months ago

Hi, I found if the message is nested, the Integer fields are recognised as strings. Could you help to solve this problem?

Here is my proto file example

syntax = "proto2";
package test;

option go_package = ".;test";

message AddFriendReq {
    repeated string phone = 1;
    optional string keyword =2;
    optional uint32 number = 3;
    optional Pagination pagination = 4;
}

message Pagination {
    optional string cursor = 1; // cursor for current page
    optional int64 limit = 2; // page size
    optional string next = 3; // cursor for next page in response, nil means no next page
}

as you can see, the Pagination message is nested in the AddFriendReq, when I set AddFriendReq like this, the Limit is set as int64(6)

func GetMessageBin() []byte {
    req := &testpb.AddFriendReq{
        Phone:   []string{"13145990022", "131313233"},
        Keyword: proto.String("I am good"),
        Number:  proto.Uint32(3),
        Pagination: &testpb.Pagination{
            Cursor: proto.String("cursor"),
            Limit:  proto.Int64(6),
        },
    }
    bin, err := proto.Marshal(req)
    if err != nil {
        fmt.Printf("bin=%v,err=%v", bin, err)
    }
    return bin
}

Then I use dmsg.Unmarshaland dmsg.MarshalJSONPB to tranform the proto data into Json string

Filename := "./proto/test.proto"
    Parser := protoparse.Parser{}
    descs, err := Parser.ParseFiles(Filename)
    if err != nil {
        fmt.Printf("ParseFiles err=%v", err)
        return
    }

    msg := descs[0].FindMessage("test.AddFriendReq")
    dmsg := dynamic.NewMessage(msg)

         // get binByte from GetMessageBin()
    binByte := GetMessageBin()
    binString := string(binByte)
    err = dmsg.Unmarshal(binByte)
    if err != nil {
        fmt.Println("err ", err)
        fmt.Println("binstring:", binString)
        fmt.Println("dmsg", dmsg)
    }

    jsStr, _ := dmsg.MarshalJSONPB(&jsonpb.Marshaler{
        OrigName: true,
    })
    fmt.Printf("jsStr=%s\n", jsStr)

Finally, the jsStr will be jsStr={"phone":["13145990022","131313233"],"keyword":"I am good","number":3,"pagination":{"cursor":"cursor","limit":"6"}}

the limit become "6", rather than int64(6)

Big-big-orange commented 7 months ago

I think I have found the root cause:

according to https://github.com/jhump/protoreflect/blob/main/dynamic/json.go#L386, if dataType=int64, it will become strings due to strconv.FormatInt(), So I have a question, why we don't use https://github.com/jhump/protoreflect/blob/main/dynamic/json.go#L399 for dataType=int64

btw, I found the similar issue for uint64, https://github.com/jhump/protoreflect/blob/main/dynamic/json.go#L403

jhump commented 7 months ago

@Big-big-orange, this is part of the spec for the JSON format of protobuf messages: https://protobuf.dev/programming-guides/proto3/#json

64-bit integers are represented using JSON strings because JSON numbers are usually stored as IEEE 64-bit floats, which cannot represent all 64-bit integers. This is related to https://github.com/protocolbuffers/protobuf/issues/12994.

Also, FWIW, I recommend users migrate their usage of "github.com/jhump/protoreflect/dynamic" to instead use "google.golang.org/protobuf/types/dynamicpb". A (hopefully soon) v2 of this repo will remove the dynamic package (and others) since this functionality is now provided by the core Protobuf runtime for Go.