proto.Marshal panic!

[ziranliu]

I found proto.Marshal() would panic in the following case:

test.proto:

syntax = "proto3";

package main;

message Foo {
}

message Bar {
    repeated Foo foos = 1;
}

run:

protoc --gofast_out=. test.proto

test.go:

package main

import (
	"fmt"

	proto "github.com/golang/protobuf/proto"
)

func main() {
	b := &Bar{
		Foos: []*Foo{nil},
	}
	_, err := proto.Marshal(b)
	fmt.Println(err)
}

run:

go run test.go test.pb.go

Then it will panic("panic: runtime error: invalid memory address or nil pointer dereference") . While the code generated by standard protobuf will returns an error("proto: repeated field Foos has nil element").

[prestonvanloon]

+1, I would expect proto.Marshal to return an error in this case.

Likewise, this should be true:

var m *pb.MyMessage 
_, err := proto.Marshal(m) 
if err == nil {
  t.Errorf("no error returned when proto.Marshal called with nil value")
}

[prestonvanloon]

After some further investigation, I think this is by design. It seems that protobuf uses some unsafe logic to achieve faster serialization times.

[oguzyildizz]

@prestonvanloon how much time not doing a nil check saves, compared to always protect against crashes?

[gurel]

Libraries should not panic the whole application when somethings go wrong, especially if it's API is returning an error.