Akegarasu / blivedm-go

go 版 b站/bilibili 直播/bili直播 弹幕库 - websocket协议
MIT License
80 stars 14 forks source link

panic caused by zero packet length #23

Closed aynakeya closed 1 week ago

aynakeya commented 1 month ago

NewPacketFromBytes should return an empty packet, if packLen is 0 or does not match the length of the data?

func NewPacketFromBytes(data []byte) Packet {
    packLen := binary.BigEndian.Uint32(data[0:4])
    // 校验包长度
    if int(packLen) != len(data) {
        log.Error("error packet")
    }
    pv := binary.BigEndian.Uint16(data[6:8])
    op := binary.BigEndian.Uint32(data[8:12])
    body := data[16:packLen]
    packet := NewPacket(pv, op, body)
    return packet
}

error trace from user

panic: runtime error: slice bounds out of range [16:0]

goroutine 48074 [running]:
github.com/AynaLivePlayer/blivedm-go/packet.NewPacketFromBytes({0xc0058eae00?, 0x0?, 0x5007d0000?})
    C:/Users/runneradmin/go/pkg/mod/github.com/!ayna!live!player/blivedm-go@v0.0.0-20240427041017-949a66917a81/packet/packet.go:63 +0x1d2
github.com/AynaLivePlayer/blivedm-go/packet.DecodePacket(...)
    C:/Users/runneradmin/go/pkg/mod/github.com/!ayna!live!player/blivedm-go@v0.0.0-20240427041017-949a66917a81/packet/packet.go:118
github.com/AynaLivePlayer/blivedm-go/client.(*Client).wsLoop(0xc0000000c0)
    C:/Users/runneradmin/go/pkg/mod/github.com/!ayna!live!player/blivedm-go@v0.0.0-20240427041017-949a66917a81/client/client.go:154 +0xc5
created by github.com/AynaLivePlayer/blivedm-go/client.(*Client).Start
    C:/Users/runneradmin/go/pkg/mod/github.com/!ayna!live!player/blivedm-go@v0.0.0-20240427041017-949a66917a81/client/client.go:186 +0x8a
Akegarasu commented 1 month ago

When does the length of a packet become 0? Could you provide some detailed scenarios? If Bilibili returns a packet with a length of 0, I think it is necessary to add extra error handling in this function.

aynakeya commented 1 month ago

tbh, I have no idea when/how the packLen become 0. I found it only because one of user's program crashed and he sent me the error log. It seems to be very rare case? I haven't encouter it myself, nor can i reproduce it.

yeah. I think it would be good to have a length check before accessing it.