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

Printing a proto breaks equality #1339

Closed
viswesh-swaminathan opened this issue Jul 15, 2021 · 5 comments
Closed

Printing a proto breaks equality #1339

viswesh-swaminathan opened this issue Jul 15, 2021 · 5 comments

Comments

@viswesh-swaminathan
Copy link

What version of protobuf and what language are you using?
Version v1.5.2

What did you do?

https://play.golang.org/p/ZwLkLcH7Ci_w

package main

import (
	"fmt"
	"log"
	"reflect"
	"time"
	"github.com/golang/protobuf/ptypes"
)

func main() {
	now := time.Now()
	
	pbt1, err := ptypes.TimestampProto(now)
	if err != nil {
		log.Fatal(err)
	} 
	
	pbt2, err := ptypes.TimestampProto(now)
	if err != nil {
		log.Fatal(err)
	}
	
	if !reflect.DeepEqual(pbt1, pbt2) {
		log.Fatal("protos not equal")
	}
	
	fmt.Printf("%s", pbt1.String())
	
	if !reflect.DeepEqual(pbt1, pbt2) {
		log.Fatal("protos not equal after printing proto")
	}
}

What did you expect to see?

Protos should be still be equal after printing.

What did you see instead?

Protos are not equal after calling String().

Make sure you include information that can help us debug (full error message, exception listing, stack trace, logs).

Anything else we should know about your project / environment?

@puellanivis
Copy link
Collaborator

This is known behavior, and is intended. Protobufs have a bit of bookkeeping information that assists in keeping track of various things, for instance, unknown fields, a size cache, and some MessageSet state.

You need to check for equality based on semantics, not on a pure reflection comparison. There is a generic proto.Equal to help you out there.

@viswesh-swaminathan
Copy link
Author

Thanks this makes sense. Unfortunately test helpers like mockgen use reflect.DeepEqual. It would be nice if reflect.DeepEqual can fall back to generic proto.Equal for proto struct.

@Rajan-226
Copy link

@puellanivis @viswesh-swaminathan Are you also aware about how printing is just changing this bookkeeping information due to which reflect.DeepEqual is failing?

I ran into similar issue, where I also observed printing both proto messages once will make them equal forever after that.
golang/go#67169

I went to String() method of structpb.Value, but couldn't find anything where we are changing original message.
But one thing I found out is code was breaking at IsValid check in reflect package where it's trying to check zero value.

@derekperkins
Copy link

derekperkins commented May 4, 2024

printing purposefully adds randomness

@neild
Copy link
Contributor

neild commented May 4, 2024

Are you also aware about how printing is just changing this bookkeeping information due to which reflect.DeepEqual is failing?

I'm afraid reflect.DeepEqual does not work on protobuf messages. You need to use proto.Equal, or the protocmp package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants