freeconf / yang

Standards-based management for Golang microservices
Apache License 2.0
38 stars 15 forks source link

fix: fix error handling when inserting list item without key #111

Closed sibuk-harabudjasim closed 2 months ago

sibuk-harabudjasim commented 2 months ago

Encountered problem handling error when trying to add list item with erroneous key. Setup:

type ListItem struct { K int V string }

type TestContainer struct { Testl bool Testll []*ListItem }


Running below cURL:
`curl -XPOST localhost/restconf/data/test-tree:testc/testll -d '{"testll": [{"d": 2, "v": "data"}]}'`
Panics:

2024/07/09 13:55:24 http: panic serving [::1]:52775: runtime error: invalid memory address or nil pointer dereference goroutine 34 [running]: net/http.(conn).serve.func1() /opt/homebrew/Cellar/go/1.22.4/libexec/src/net/http/server.go:1898 +0xb0 panic({0x1048ab3c0?, 0x104bc5d10?}) /opt/homebrew/Cellar/go/1.22.4/libexec/src/runtime/panic.go:770 +0x124 github.com/freeconf/yang/nodeutil.(sliceAsList).findByKey(0x140000a0720, {0x10491ff30, 0x140001a6160}, {0x140000a23a0, 0x1, 0x1041edebc?}, {0x14000110430, 0x1, 0x1}) /Users/manitou/dev/test/freeconf/yang/nodeutil/node_slice.go:97 +0x2f0 github.com/freeconf/yang/nodeutil.(sliceAsList).getByKey(0x1041edf30?, {{0x140000b0370, 0x140000a0a50, 0x0, 0x140000a0810}, 0x140000b0410, 0x0, 0x0, 0x0, 0x0, ...}) /Users/manitou/dev/test/freeconf/yang/nodeutil/node_slice.go:27 +0x40 github.com/freeconf/yang/nodeutil.(Node).DoGetByKey(0x140000d6140, {{0x140000b0370, 0x140000a0a50, 0x0, 0x140000a0810}, 0x140000b0410, 0x0, 0x0, 0x0, 0x0, ...}) /Users/manitou/dev/test/freeconf/yang/nodeutil/node.go:619 +0x58 github.com/freeconf/yang/nodeutil.(Node).Next(0x0?, {{0x140000b0370, 0x140000a0a50, 0x0, 0x140000a0810}, 0x140000b0410, 0x0, 0x0, 0x0, 0x0, ...}) /Users/manitou/dev/test/freeconf/yang/nodeutil/node.go:221 +0x1ac github.com/freeconf/yang/nodeutil.dump.Node.func4({{0x140000b0370, 0x140000a0a50, 0x0, 0x140000a0810}, 0x140000b0410, 0x0, 0x0, 0x0, 0x0, 0x0, ...}) /Users/manitou/dev/test/freeconf/yang/nodeutil/dump.go:136 +0x1ac github.com/freeconf/yang/nodeutil.(Basic).Next(0x104130d2c?, {{0x140000b0370, 0x140000a0a50, 0x0, 0x140000a0810}, 0x140000b0410, 0x0, 0x0, 0x0, 0x0, ...}) /Users/manitou/dev/test/freeconf/yang/nodeutil/basic.go:172 +0x52c github.com/freeconf/yang/node.(*Selection).selectListItem(0x140000b0370, 0x140000aa200)



This PR adds extra validation. To both cases, items list and items map. Please, review and merge if it is OK for you.
For me, not sure about 2 things here:
 - `r.Key == nil` seems like some legacy, because `r.Key` is a slice
 - did not find any tests to take as example to test this case
sibuk-harabudjasim commented 2 months ago

thanks for the PR! 1 comment. Did unit tests all pass?

Yes, tests are passing