planetscale / planetscale/vtprotobuf
Nil pointer panic marshalling nil oneof field
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 1.1k
- Forks
- 112
- PR merge metrics
- No merged PRs in 30d
Description
Hi 👋 Ran into a bit of a weird one. Given the following proto:
syntax = "proto3";
package proto;
option go_package = "github.com/pfouilloux/vttest/proto";
message TestMsg {
oneof Test {
string a = 1;
string b = 2;
string c = 3;
}
}
and the following code:
package main_test
import (
"testing"
"github.com/planetscale/vtprotobuf/codec/grpc"
_ "google.golang.org/grpc/encoding/proto"
"vttest/proto/github.com/pfouilloux/vttest/proto"
)
//go:generate protoc --proto_path=proto --go_out=proto --go_opt=paths=source_relative --go-vtproto_out=proto --go-vtproto_opt=features=marshal+unmarshal+size oneof.proto
func TestMarshal(t *testing.T) {
test := &proto.TestMsg{Test: getA()}
_, err := grpc.Codec{}.Marshal(test)
if err != nil {
panic(err)
}
}
func getA() *proto.TestMsg_A {
return nil
}
I'm seeing the following error:
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x1239f1d]
goroutine 4 [running]:
testing.tRunner.func1.2({0x126e880, 0x14b3f20})
/usr/local/opt/go/libexec/src/testing/testing.go:1396 +0x24e
testing.tRunner.func1()
/usr/local/opt/go/libexec/src/testing/testing.go:1399 +0x39f
panic({0x126e880, 0x14b3f20})
/usr/local/opt/go/libexec/src/runtime/panic.go:884 +0x212
vttest/proto/github.com/pfouilloux/vttest/proto.(*TestMsg_A).MarshalToSizedBufferVT(0x126e580?, {0x14efc18?, 0xc000057601?, 0x0?})
/Users/pfouilloux/code/vttest/proto/github.com/pfouilloux/vttest/proto/oneof_vtproto.pb.go:72 +0x1d
vttest/proto/github.com/pfouilloux/vttest/proto.(*TestMsg_A).MarshalToVT(0x1240e01?, {0x14efc18?, 0x0?, 0x123a5a4?})
/Users/pfouilloux/code/vttest/proto/github.com/pfouilloux/vttest/proto/oneof_vtproto.pb.go:68 +0x6a
vttest/proto/github.com/pfouilloux/vttest/proto.(*TestMsg).MarshalToSizedBufferVT(0xc0001049c0?, {0x14efc18, 0x0, 0x0})
/Users/pfouilloux/code/vttest/proto/github.com/pfouilloux/vttest/proto/oneof_vtproto.pb.go:58 +0x133
vttest/proto/github.com/pfouilloux/vttest/proto.(*TestMsg).MarshalVT(0xc0001049c0)
/Users/pfouilloux/code/vttest/proto/github.com/pfouilloux/vttest/proto/oneof_vtproto.pb.go:27 +0x58
github.com/planetscale/vtprotobuf/codec/grpc.Codec.Marshal({}, {0x12a20c0, 0xc0001049c0})
/Users/pfouilloux/go/pkg/mod/github.com/planetscale/vtprotobuf@v0.3.0/codec/grpc/grpc_codec.go:20 +0x42
vttest_test.TestMarshal(0x0?)
/Users/pfouilloux/code/vttest/oneof_test.go:15 +0x47
testing.tRunner(0xc0000076c0, 0x12c8b20)
/usr/local/opt/go/libexec/src/testing/testing.go:1446 +0x10b
created by testing.(*T).Run
/usr/local/opt/go/libexec/src/testing/testing.go:1493 +0x35f
Process finished with the exit code 1
It looks like there is a nil check missing in the implementation of MarshalToVT for *TestMsg_A
func (m *TestMsg_A) MarshalToVT(dAtA []byte) (int, error) {
size := m.SizeVT()
return m.MarshalToSizedBufferVT(dAtA[:size])
}
I'm more than happy to raise a PR to address this if you could give me some guidance on where to add the appropriate tests.
Kind regards & thanks for sharing your work with the community!
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with oneof_test.go and the generated oneof_vtproto.pb.go, especially MarshalToVT and MarshalToSizedBufferVT; then trace the go:generate command to the generator source. Add a regression test for the nil TestMsg_A case and run TestMarshal or the relevant Go tests; done means marshalling no longer panics.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100