Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XXX_Marshal returns wrong encoded byte slice #863

Closed
zqkgo opened this issue Jun 1, 2019 · 1 comment
Closed

XXX_Marshal returns wrong encoded byte slice #863

zqkgo opened this issue Jun 1, 2019 · 1 comment

Comments

@zqkgo
Copy link

zqkgo commented Jun 1, 2019

What version of protobuf and what language are you using?
protobuf version:

$ protoc --version
libprotoc 3.7.1

language: Go

What did you do?
Step 1
define test.proto file

syntax = "proto3";
package proto_test;

message Education {
    string college = 1;
}

message Person {
    int32 age = 1;
    string name = 2;
    Education edu = 3;
}

Step 2

generate Go code

protoc --go_out=. test.proto

Step 3

check encoded message

p := proto_test.Person{
    Age:  666,
    Name: "Tom",
    Edu: &proto_test.Education{
        College: "SOMEWHERE",
    },
}
var b []byte
out, err := p.XXX_Marshal(b, true)
if err != nil {
    log.Fatalln("fail to marshal with error: ", err)
}
fmt.Printf("hexadecimal format:% x \n", out)

What did you expect to see?

it should output,

hexadecimal format:08 9a 05 12 03 54 6f 6d 1a 0b 0a 09 53 4f 4d 45 57 48 45 52 45 

What did you see instead?

but it outputs,

hexadecimal format:08 9a 05 12 03 54 6f 6d 1a dd 97 d1 08 0a 09 53 4f 4d 45 57 48 45 52 45 

the dd 97 d1 08 part should be 0b.

I debug the process of encoding Person struct, here, when Education struct is marshaled, u.cachedsize() tries to check if the sizecache field is valid, when value of u.sizecache is zeroField it assumes valid, then it returns the wrong int which starts at offset 0. In this case, u.sizecache should be 40, and p.Ed.XXX_sizecache is 11 which encoded as 0b.

@dsnet
Copy link
Member

dsnet commented Jun 1, 2019

Hi, thanks for the report.

The XXX methods and fields are not to be interacted with directly. As you have correctly noted, XXX_Marshal makes assumptions about the validity of XXX_sizecache. This is working as intended. Instead, the proto.Marshal function should be used.

I should note that the compatibility section reserves the right to add/remove XXX methods. In fact, they will be going away in the future. See #276.

@dsnet dsnet closed this as completed Jun 1, 2019
@golang golang locked and limited conversation to collaborators Jun 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants