gorilla / schema

Package gorilla/schema fills a struct with form values.
https://gorilla.github.io
BSD 3-Clause "New" or "Revised" License
1.38k stars 229 forks source link

[BUG] panic in 1.3.0 #213

Closed davidnewhall closed 4 months ago

davidnewhall commented 6 months ago

Is there an existing issue for this?

Current Behavior

upgraded to 1.3.0 and got screenfuls of panic messages when decoding values.

[ERROR] 2024/04/07 14:20:59 http: panic serving 127.0.0.1:54990: reflect: indirection through nil pointer to embedded struct
goroutine 411 [running]:
net/http.(*conn).serve.func1()
    net/http/server.go:1898 +0xbe
panic({0xc1fe80?, 0xfd37b0?})
    runtime/panic.go:770 +0x132
reflect.Value.FieldByIndex({0xd35160?, 0xc000a8bd18?, 0x0?}, {0xc0008cd5e0, 0x2, 0xd0ef20?})
    reflect/value.go:1324 +0x174
reflect.Value.FieldByName({0xd35160?, 0xc000a8bd18?, 0xd35160?}, {0xba64a0?, 0xc000a8bcf8?})
    reflect/value.go:1363 +0x14c
github.com/gorilla/schema.(*Decoder).setDefaults(0x1b32c10, {0xfe8868, 0xd35160}, {0xd35160?, 0xc000a8bd18?, 0xc000ffbc08?})
    github.com/gorilla/schema@v1.3.0/decoder.go:108 +0x1f4
github.com/gorilla/schema.(*Decoder).setDefaults(0x1b32c10, {0xfe8868, 0xd2cb60}, {0xd2cb60?, 0xc000a8bce0?, 0x22?})
    github.com/gorilla/schema@v1.3.0/decoder.go:111 +0x2c5
github.com/gorilla/schema.(*Decoder).setDefaults(0x1b32c10, {0xfe8868, 0xd5ee60}, {0xd5ee60?, 0xc00096f080?, 0x10?})
    github.com/gorilla/schema@v1.3.0/decoder.go:113 +0x3d2
github.com/gorilla/schema.(*Decoder).setDefaults(0x1b32c10, {0xfe8868, 0xd73a00}, {0xd73a00?, 0xc0006748c0?, 0xc0006748c0?})
    github.com/gorilla/schema@v1.3.0/decoder.go:113 +0x3d2
github.com/gorilla/schema.(*Decoder).Decode(0x1b32c10, {0xd61320?, 0xc0006748c0?}, 0xc000e5f170)
    github.com/gorilla/schema@v1.3.0/decoder.go:87 +0x3fb
github.com/Notifiarr/notifiarr/pkg/client.(*Client).mergeAndValidateNewConfig(0xc0002880b0, 0xc0006748c0, 0xc001250360)

Expected Behavior

1.2.1 didn't do this. I don't think I've set any default tags.

Steps To Reproduce

Not real sure other than clone Notifiarr/notifiarr and go build it. Then run it, log in, and make a configuration change. That probably sucks, so I'm hopeful the stack trace is sufficient.

Anything else?

No response

zak905 commented 6 months ago

Hi @davidnewhall, I am the contributor of the default functionality that was merged recently - I will look into this asap. There is a PR #211 that also speaks about a nil pointer issue, not sure it's related

davidnewhall commented 6 months ago

Thank you. Looks like it could be the same issue.

hidetatz commented 4 months ago

Hi maintainers, this issue seems to be fixed by the above pull request. Would you kindly create a new release including it?

jaitaiwan commented 4 months ago

Hey we plan on doing a release soon, we're just awaiting a fix for a low-level security issue and then intend on releasing after that.

jaitaiwan commented 4 months ago

v1.4.0 has been released containing this fix. Thanks

hidetatz commented 4 months ago

Thank you!