divide by zero panic in hexdump when ConfigState.BytesWidth == 0

[mattsawyer77]

We encountered this error with utter v1.5.0. In the process of migrating from spew to utter, we were constructing ConfigState without specifying any width values. After seeing this we realized we needed to use ConfigState.NewDefaultConfig() (or set valid values for the config fields). I thought I would mention it, in case the maintainers wanted to validate the config before trying to serialize the data.

stack trace:

panic({0x86b3a60, 0x10164560})
        /usr/lib/golang/src/runtime/panic.go:884 +0x213
github.com/kortschak/utter.hexDump({0x7f92b8adc470, 0xc0486ea980}, {0xc0123a69a0, 0x14b, 0x14b?}, {0xc0486eadf8, 0x6}, 0x0, 0x0)
        <redacted>vendor/github.com/kortschak/utter/common.go:239 +0x5cf
github.com/kortschak/utter.(*dumpState).dumpSlice(0xc008213a20, {0x83d0700?, 0xc03344fc80?, 0x20?}, 0xb8?)
        <redacted>vendor/github.com/kortschak/utter/dump.go:303 +0x651
github.com/kortschak/utter.(*dumpState).dump(0xc008213a20, {0x83d0700?, 0xc03344fc80?, 0x0?}, 0x0, 0x1, 0x0, 0xc03344fc80)
        <redacted>vendor/github.com/kortschak/utter/dump.go:465 +0x144a
github.com/kortschak/utter.(*dumpState).dump(0xc008213a20, {0x8bff200?, 0xc03344fc70?, 0x20?}, 0x1, 0x0, 0x0, 0xc03344fc70)
        <redacted>vendor/github.com/kortschak/utter/dump.go:558 +0x1272
github.com/kortschak/utter.(*dumpState).dumpPtr(0xc008213a20, {0x8db7620?, 0xc024fcdd50?, 0xc008211218?})
        <redacted>vendor/github.com/kortschak/utter/dump.go:210 +0xacc
github.com/kortschak/utter.(*dumpState).dump(0xc008213a20, {0x8db7620?, 0xc024fcdd50?, 0x0?}, 0x58?, 0x25?, 0x44?, 0x8db7620?)
        <redacted>vendor/github.com/kortschak/utter/dump.go:358 +0xfca
github.com/kortschak/utter.(*dumpState).dump(0xc008213a20, {0x879cbc0?, 0xc024fcdd50?, 0x30?}, 0x1, 0x0, 0x0, 0xc024fcdd50)
        <redacted>vendor/github.com/kortschak/utter/dump.go:558 +0x1272
github.com/kortschak/utter.(*dumpState).dumpPtr(0xc008213a20, {0x8dbb360?, 0xc024fcdd48?, 0xc008211b40?})
        <redacted>vendor/github.com/kortschak/utter/dump.go:210 +0xacc
github.com/kortschak/utter.(*dumpState).dump(0xc008213a20, {0x8dbb360?, 0xc024fcdd48?, 0x0?}, 0x58?, 0x25?, 0x44?, 0x8dbb360?)
        <redacted>vendor/github.com/kortschak/utter/dump.go:358 +0xfca
github.com/kortschak/utter.(*dumpState).dump(0xc008213a20, {0x879cc40?, 0xc024fcdd48?, 0x20?}, 0x1, 0x0, 0x0, 0xc024fcdd48)
        <redacted>vendor/github.com/kortschak/utter/dump.go:558 +0x1272
github.com/kortschak/utter.(*dumpState).dumpPtr(0xc008213a20, {0x8dbb520?, 0xc032b67798?, 0xc008212468?})
        <redacted>vendor/github.com/kortschak/utter/dump.go:210 +0xacc
github.com/kortschak/utter.(*dumpState).dump(0xc008213a20, {0x8dbb520?, 0xc032b67798?, 0x0?}, 0x58?, 0x25?, 0x44?, 0x8dbb520?)
        <redacted>vendor/github.com/kortschak/utter/dump.go:358 +0xfca
github.com/kortschak/utter.(*dumpState).dump(0xc008213a20, {0x8a544a0?, 0xc032b67788?, 0x20?}, 0x1, 0x0, 0x0, 0xc032b67788)
        <redacted>vendor/github.com/kortschak/utter/dump.go:558 +0x1272
github.com/kortschak/utter.(*dumpState).dumpPtr(0xc008213a20, {0x8e3c160?, 0xc032b677e8?, 0xc008212d90?})
        <redacted>vendor/github.com/kortschak/utter/dump.go:210 +0xacc
github.com/kortschak/utter.(*dumpState).dump(0xc008213a20, {0x8e3c160?, 0xc032b677e8?, 0x0?}, 0x58?, 0x25?, 0x44?, 0x8e3c160?)
        <redacted>vendor/github.com/kortschak/utter/dump.go:358 +0xfca
github.com/kortschak/utter.(*dumpState).dump(0xc008213a20, {0x8e6cf40?, 0xc032b677e8?, 0x20?}, 0x1, 0x0, 0x0, 0xc032b677e8)
        <redacted>vendor/github.com/kortschak/utter/dump.go:558 +0x1272
github.com/kortschak/utter.(*dumpState).dumpPtr(0xc008213a20, {0x90f10e0?, 0xc032b677e8?, 0x5cb1bf?})
        <redacted>vendor/github.com/kortschak/utter/dump.go:210 +0xacc
github.com/kortschak/utter.(*dumpState).dump(0xc008213a20, {0x90f10e0?, 0xc032b677e8?, 0x41350a?}, 0xc0?, 0xfc?, 0x44?, 0x50?)
        <redacted>vendor/github.com/kortschak/utter/dump.go:358 +0xfca
github.com/kortschak/utter.fdump(0xc03344fcc0, {0x7f92b8adc470?, 0xc0486ea980?}, {0x90f10e0, 0xc032b677e8})
        <redacted>vendor/github.com/kortschak/utter/dump.go:763 +0x1d9
github.com/kortschak/utter.(*ConfigState).Fdump(...)
        <redacted>vendor/github.com/kortschak/utter/config.go:126

[kortschak]

Thanks. The three solutions that I see for this are to: not line-fold if bytes width is zero, panic on call of hexDump if it is zero as now but with an informative message, or to put a reasonable value into the var if it's zero (what is reasonable).

My personal preference would be to panic with an informative message since this is a programmer error. What is your view?

By comparison, spew calls hex.Dump which matches hexdump -C which has a width of 16, so I guess that would answer the question of "What is reasonable?" if that route is taken.

[mattsawyer77]

My personal preference would be to panic with an informative message since this is a programmer error. What is your view?

Because this is hard to repro (i.e., it only panics on some data structures where the internal logic decides to do a hexDump), it may catch users off-guard at runtime. Our automated tests never hit this condition; we unfortunately didn't catch it until our app crashed in a real environment. If it crashed unconditionally, I'd agree with you.

IMHO I'd prefer either not line-folding if width is unset, or (if that is considered a silently breaking change to existing users), panicking immediately on any of the following if the config is invalid:

• (*ConfigState).Dump
• (*ConfigState).Fdump
• (*ConfigState).Sdump

Or if those options are unpalatable, you could deprecate the public ConfigState struct and add a config type with private fields that is configured with functional options, thereby making invalid configs impossible to construct. just a thought.

[kortschak]

I have #35 (this will be 1.7.0 after it's merged). This sets the width to 16 if it's unset or negative. I took a look at making the output unfolded, and I don't particularly like it; it's harder to do and given the kinds of values that []byte can hold may make the output unusable due to line length.

I decided against a panic (it would not have been immediately on the call to the exported function since width is not necessarily used, but would have been in the preamble to hexDump where the change in #35 is) as I think we probably should make the zero config usable.