search

godbus / dbus

Native Go bindings for D-Bus
BSD 2-Clause "Simplified" License
959 stars 222 forks source link

Panic in decoding #376

Closed raas closed 8 months ago

raas commented 10 months ago

Hi,

Is there any reason why encoder.go / decoder.go should ever call panic()? Panicking on user input is generally frowned upon -- I can't handle it in the application without second-guessing the decoding. Returning an error would allow for graceful handling.

(thanks for this project of awesome name btw, I'm writing https://gitlab.com/andras.horvath1/notify-proxy, and I might be holding things wrong)

2023/09/14 14:26:59 could not decode message: dbus: wire format error: invalid value for boolean
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x1 pc=0x7f60a6]

goroutine 28 [running]:
github.com/godbus/dbus/v5.(*Message).validateHeader(0x0?)
    /home/ahorvath/go/pkg/mod/github.com/godbus/dbus/v5@v5.1.0/message.go:294 +0x26
github.com/godbus/dbus/v5.(*Message).EncodeToWithFDs(0x0, {0x9b73a0, 0xc000296900}, {0x9bd0b0?, 0xce77d0})
    /home/ahorvath/go/pkg/mod/github.com/godbus/dbus/v5@v5.1.0/message.go:230 +0x5e
github.com/godbus/dbus/v5.(*Message).EncodeTo(...)
    /home/ahorvath/go/pkg/mod/github.com/godbus/dbus/v5@v5.1.0/message.go:282
github.com/godbus/dbus/v5.(*Message).IsValid(...)
    /home/ahorvath/go/pkg/mod/github.com/godbus/dbus/v5@v5.1.0/message.go:290
github.com/godbus/dbus/v5.(*Message).String(0x0)
    /home/ahorvath/go/pkg/mod/github.com/godbus/dbus/v5@v5.1.0/message.go:351 +0x5f
main.send(0x90c257?)
    /home/ahorvath/Private/notify-proxy/notifications.go:27 +0x4b
miccoli commented 9 months ago

2023/09/14 14:26:59 could not decode message: dbus: wire format error: invalid value for boolean panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x1 pc=0x7f60a6]

Here it seems that a runtime error is raised, not a panic call from godbus. I had similar problems... are you shure that the *Message pointer is properly initialized?

In fact

package main

import (
    "fmt"

    "github.com/godbus/dbus/v5"
)

func main() {
    var msg *dbus.Message
    fmt.Println(msg.String())
}

fails with

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x1 pc=0x4d58fa]

goroutine 1 [running]:
github.com/godbus/dbus/v5.(*Message).validateHeader(0xc000074e00?)
    /tmp/gopath2268097760/pkg/mod/github.com/godbus/dbus/v5@v5.1.0/message.go:294 +0x1a
github.com/godbus/dbus/v5.(*Message).EncodeToWithFDs(0x0, {0x53c0c0, 0xc000016450}, {0x53ce28?, 0x612da0})
    /tmp/gopath2268097760/pkg/mod/github.com/godbus/dbus/v5@v5.1.0/message.go:230 +0x4f
github.com/godbus/dbus/v5.(*Message).EncodeTo(...)
    /tmp/gopath2268097760/pkg/mod/github.com/godbus/dbus/v5@v5.1.0/message.go:282
github.com/godbus/dbus/v5.(*Message).IsValid(...)
    /tmp/gopath2268097760/pkg/mod/github.com/godbus/dbus/v5@v5.1.0/message.go:290
github.com/godbus/dbus/v5.(*Message).String(0x0)
    /tmp/gopath2268097760/pkg/mod/github.com/godbus/dbus/v5@v5.1.0/message.go:351 +0x53
main.main()
    /tmp/sandbox654772962/prog.go:11 +0x15

You can try it at https://go.dev/play/p/_cmzclBLWJq.

raas commented 8 months ago

Hm, indeed, I had missed a nil check before calling msg.String() ... thanks :)