Clone and Merge panic with nonnullable stdtime types #519

vgough · 2018-12-01T01:27:07Z

When a message has a non-nullable stdtime type, eg:

    message Test {
        google.protobuf.Timestamp when = 1 [
            (gogoproto.stdtime) = true,
            (gogoproto.nullable) = false
        ];
    }

Then proto.Merge and proto.Clone both panic. Are they trying to reflect walk into the time.Time type even though it's not a proto type?

    panic: merger not found for type:int

    vendor/github.com/gogo/protobuf/proto.(*mergeInfo).computeMergeInfo(0xc0000bb940)
    vendor/github.com/gogo/protobuf/proto/table_merge.go:643 +0xe30
    vendor/github.com/gogo/protobuf/proto.(*mergeInfo).merge(0xc0000bb940, 0xc0000a2d90, 0x1d49910)
    vendor/github.com/gogo/protobuf/proto/table_merge.go:113 +0x4d5
    vendor/github.com/gogo/protobuf/proto.(*mergeInfo).computeMergeInfo.func27(0xc0000a2d90, 0x1d49910)
    vendor/github.com/gogo/protobuf/proto/table_merge.go:536 +0x3e
    vendor/github.com/gogo/protobuf/proto.(*mergeInfo).merge(0xc0000bb900, 0xc0000a2d80, 0x1d49900)
    vendor/github.com/gogo/protobuf/proto/table_merge.go:139 +0x49d
    vendor/github.com/gogo/protobuf/proto.(*mergeInfo).computeMergeInfo.func29(0xc000144528, 0xc0001444a8)
    vendor/github.com/gogo/protobuf/proto/table_merge.go:568 +0x5a
    vendor/github.com/gogo/protobuf/proto.(*mergeInfo).merge(0xc0000bb880, 0xc000144518, 0xc000144498)
    vendor/github.com/gogo/protobuf/proto/table_merge.go:139 +0x49d
    vendor/github.com/gogo/protobuf/proto.(*mergeInfo).computeMergeInfo.func27(0xc000144518, 0xc000144498)
    vendor/github.com/gogo/protobuf/proto/table_merge.go:536 +0x3e
    vendor/github.com/gogo/protobuf/proto.(*mergeInfo).merge(0xc0000bb780, 0xc000144500, 0xc000144480)
    vendor/github.com/gogo/protobuf/proto/table_merge.go:139 +0x49d
    vendor/github.com/gogo/protobuf/proto.(*InternalMessageInfo).Merge(0x1d46f60, 0x182c5e0, 0xc000144500, 0x182c5e0, 0xc000144480)
    vendor/github.com/gogo/protobuf/proto/table_merge.go:50 +0x54
    test.(*Test).XXX_Merge(0xc000144500, 0x182c5e0, 0xc000144480)
    
    vendor/github.com/gogo/protobuf/proto.Merge(0x182c5e0, 0xc000144500, 0x182c5e0, 0xc000144480)

The text was updated successfully, but these errors were encountered:

RaduBerinde · 2018-12-13T15:55:03Z

I am working on this.

RaduBerinde · 2018-12-13T15:56:08Z

Hm, actually, never mind, I think this might be a different issue than repeated non-nullable structs (which is what I'm working on).

ethanfrey · 2019-07-11T15:05:12Z

I came across a similar issue.
When I cast the times to UTC before running Clone, all tests passed (no more panic)

t.When = t.When.UTC()

Maybe there is a better solution, but this is the best workaround I could find

gogo/protobuf#519 Seems like the old code never properly cloned this, which was not easily detectable through so many layers of indirection

The following panic can be observed when calling `api/v1/rules` without this change: ``` 2020/07/22 21:44:22 http: panic serving 127.0.0.1:49368: merger not found for type:int goroutine 529 [running]: net/http.(*conn).serve.func1(0xc00029e640) /usr/lib/go-1.14/src/net/http/server.go:1772 +0x139 panic(0x1e0a400, 0xc000368c30) /usr/lib/go-1.14/src/runtime/panic.go:975 +0x3e3 github.com/gogo/protobuf/proto.(*mergeInfo).computeMergeInfo(0xc0008b8780) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:662 +0xfc6 github.com/gogo/protobuf/proto.(*mergeInfo).merge(0xc0008b8780, 0xc0007447a0, 0xc00057bb00) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:113 +0x4a2 github.com/gogo/protobuf/proto.(*mergeInfo).computeMergeInfo.func27(0xc000091b70, 0x3991910) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:545 +0x381 github.com/gogo/protobuf/proto.(*mergeInfo).merge(0xc0008b8700, 0xc000091b60, 0x3991900) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:139 +0x46a github.com/gogo/protobuf/proto.(*mergeInfo).computeMergeInfo.func30(0xc00061e470, 0xc00061e370) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:587 +0x5c github.com/gogo/protobuf/proto.(*mergeInfo).merge(0xc0008b8340, 0xc00061e460, 0xc00061e360) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:139 +0x46a github.com/gogo/protobuf/proto.(*mergeInfo).computeMergeInfo.func28(0xc00061e460, 0xc00061e360) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:555 +0x3e github.com/gogo/protobuf/proto.(*mergeInfo).merge(0xc0008b83c0, 0xc00061e400, 0xc00061e300) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:139 +0x46a github.com/gogo/protobuf/proto.(*InternalMessageInfo).Merge(0x398a440, 0x2676ea0, 0xc00061e400, 0x2676ea0, 0xc00061e300) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:50 +0x4c github.com/thanos-io/thanos/pkg/rules/rulespb.(*RecordingRule).XXX_Merge(0xc00061e400, 0x2676ea0, 0xc00061e300) /home/gstatkevicius/dev/thanos/pkg/rules/rulespb/rpc.pb.go:527 +0x57 ``` Let's use the workaround given here: gogo/protobuf#519. Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com>

* rules: manager: avoid panic when cloning The following panic can be observed when calling `api/v1/rules` without this change: ``` 2020/07/22 21:44:22 http: panic serving 127.0.0.1:49368: merger not found for type:int goroutine 529 [running]: net/http.(*conn).serve.func1(0xc00029e640) /usr/lib/go-1.14/src/net/http/server.go:1772 +0x139 panic(0x1e0a400, 0xc000368c30) /usr/lib/go-1.14/src/runtime/panic.go:975 +0x3e3 github.com/gogo/protobuf/proto.(*mergeInfo).computeMergeInfo(0xc0008b8780) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:662 +0xfc6 github.com/gogo/protobuf/proto.(*mergeInfo).merge(0xc0008b8780, 0xc0007447a0, 0xc00057bb00) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:113 +0x4a2 github.com/gogo/protobuf/proto.(*mergeInfo).computeMergeInfo.func27(0xc000091b70, 0x3991910) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:545 +0x381 github.com/gogo/protobuf/proto.(*mergeInfo).merge(0xc0008b8700, 0xc000091b60, 0x3991900) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:139 +0x46a github.com/gogo/protobuf/proto.(*mergeInfo).computeMergeInfo.func30(0xc00061e470, 0xc00061e370) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:587 +0x5c github.com/gogo/protobuf/proto.(*mergeInfo).merge(0xc0008b8340, 0xc00061e460, 0xc00061e360) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:139 +0x46a github.com/gogo/protobuf/proto.(*mergeInfo).computeMergeInfo.func28(0xc00061e460, 0xc00061e360) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:555 +0x3e github.com/gogo/protobuf/proto.(*mergeInfo).merge(0xc0008b83c0, 0xc00061e400, 0xc00061e300) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:139 +0x46a github.com/gogo/protobuf/proto.(*InternalMessageInfo).Merge(0x398a440, 0x2676ea0, 0xc00061e400, 0x2676ea0, 0xc00061e300) /home/gstatkevicius/go/pkg/mod/github.com/gogo/protobuf@v1.3.1/proto/table_merge.go:50 +0x4c github.com/thanos-io/thanos/pkg/rules/rulespb.(*RecordingRule).XXX_Merge(0xc00061e400, 0x2676ea0, 0xc00061e300) /home/gstatkevicius/dev/thanos/pkg/rules/rulespb/rpc.pb.go:527 +0x57 ``` Let's use the workaround given here: gogo/protobuf#519. Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com> * rules: manager: cover all cases Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com>

gogo/protobuf#519

* Add Plugin resource schema, methods * Improve shebang of genproto.sh Execute using bash, no matter where it actually lives * Use Metadata.Expiry() Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com> * Remove field reservations from PluginStatusCode * Add plugin (un)marshaling * Snake case fields of Plugin (and children) * Ensure timestamp fields on Plugin are always UTC gogo/protobuf#519 * Rename credentials according to proto conventions * Fold check for nil settings into the type switch * Remove extraneous field checks These are set in setStaticFields() * Add missing godocs --------- Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com>

* Add Plugin resource schema, methods * Improve shebang of genproto.sh Execute using bash, no matter where it actually lives * Use Metadata.Expiry() * Remove field reservations from PluginStatusCode * Add plugin (un)marshaling * Snake case fields of Plugin (and children) * Ensure timestamp fields on Plugin are always UTC gogo/protobuf#519 * Rename credentials according to proto conventions * Fold check for nil settings into the type switch * Remove extraneous field checks These are set in setStaticFields() * Add missing godocs --------- Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com>

ethanfrey added a commit to CosmWasm/cosmos-sdk that referenced this issue Jul 11, 2019

Fix Clone by forcing all Header.Time in Context to be UTC

a0f6e4d

gogo/protobuf#519 Seems like the old code never properly cloned this, which was not easily detectable through so many layers of indirection

GiedriusS mentioned this issue Jul 22, 2020

rules: manager: avoid panic when cloning when TZ != UTC thanos-io/thanos#2925

Merged

2 tasks

a-palchikov mentioned this issue Feb 24, 2021

panic in proto merge when deep-copying server value gravitational/teleport#5691

Closed

espadolini mentioned this issue Apr 19, 2022

Fix panic in CertAuthority.Clone because of non-UTC times. gravitational/teleport#12057

Merged

justinas added a commit to gravitational/teleport that referenced this issue Feb 1, 2023

Ensure timestamp fields on Plugin are always UTC

2fe8701

gogo/protobuf#519

tobiaszheller mentioned this issue Apr 27, 2023

Fix dynamo TestEmitAuditEventForLargeEvents gravitational/teleport#25256

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clone and Merge panic with nonnullable stdtime types #519

Clone and Merge panic with nonnullable stdtime types #519

vgough commented Dec 1, 2018

RaduBerinde commented Dec 13, 2018

RaduBerinde commented Dec 13, 2018

ethanfrey commented Jul 11, 2019

Clone and Merge panic with nonnullable stdtime types #519

Clone and Merge panic with nonnullable stdtime types #519

Comments

vgough commented Dec 1, 2018

RaduBerinde commented Dec 13, 2018

RaduBerinde commented Dec 13, 2018

ethanfrey commented Jul 11, 2019