freeconf / yang

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

Explicit implementation of 'OnGetByKey' handler, causes panic during delete node operation if 'OnDeleteByKey' is not explicitly implemented #120

Open Pawel-Guzik opened 3 weeks ago

Pawel-Guzik commented 3 weeks ago

Actual behavior Explicit implementation of OnGetByKey handler within the node, causes the library panic during DELETE request handling if, OnDeleteByKey is not explicit implemented.


Expected behavior Explicit implementation of OnGetByKey handler within the node, does NOT cause the library panic during DELETE request handling, if OnDeleteByKey is not explicit implemented

Reproduction steps Code used to reproduce the bug can be found in the section below

  1. Start restconf server - go run main.go
  2. Create more than 1 resource within the books Yang node (it is already done, by configuration within startup.json file)
  3. Perform a DELETE request to delete 1 resource within the books node Request: curl -X DELETE http://localhost:8080/restconf/data/library:books=0 Response: curl: (52) Empty reply from server Server logs:

    2024/10/03 19:01:59 http: panic serving [::1]:38970: runtime error: invalid memory address or nil pointer dereference
    goroutine 8 [running]:
    net/http.(*conn).serve.func1()
    /usr/local/go/src/net/http/server.go:1947 +0xbe
    panic({0x84b180?, 0xf53b70?})
    /usr/local/go/src/runtime/panic.go:785 +0x132
    github.com/freeconf/yang/nodeutil.(*Node).Next(0x411da5?, {{0xc0000a1040, 0x0, 0x0, 0x0}, 0x0, 0x0, 0x1, 0x0, 0x0, ...})
    /home/pawel/go/pkg/mod/github.com/freeconf/yang@v0.0.0-20240126135339-ef92ddeb9f99/nodeutil/node.go:213 +0x131
    github.com/freeconf/yang/node.(*Selection).Delete(0xc0000a1090)
    /home/pawel/go/pkg/mod/github.com/freeconf/yang@v0.0.0-20240126135339-ef92ddeb9f99/node/selection.go:392 +0x1b9
    github.com/freeconf/restconf.(*browserHandler).ServeHTTP(0xc00006a358, {0x1, 0x1, 0x1, 0x1, 0x1}, {0xc80a58?, 0xc0002c42d0?}, {0xc7fef0, 0xc0002a3b20}, ...)
    /home/pawel/go/pkg/mod/github.com/freeconf/restconf@v0.0.0-20240627124255-9543e7933a6e/browser_handler.go:96 +0x8fa
    github.com/freeconf/restconf.(*Server).serve(0xc0001e3950?, {0x1, 0x1, 0x1, 0x1, 0x1}, {0xc80a58, 0xc0002c42d0}, {0xc81100, 0xc000012108}, ...)
    /home/pawel/go/pkg/mod/github.com/freeconf/restconf@v0.0.0-20240627124255-9543e7933a6e/server.go:243 +0x13f
    github.com/freeconf/restconf.(*Server).ServeHTTP(0xc0000acb40, {0xc7fef0, 0xc0002a3b20}, 0xc0002c8000)
    /home/pawel/go/pkg/mod/github.com/freeconf/restconf@v0.0.0-20240627124255-9543e7933a6e/server.go:185 +0xa9a
    net/http.serverHandler.ServeHTTP({0xc0002c4150?}, {0xc7fef0?, 0xc0002a3b20?}, 0x6?)
    /usr/local/go/src/net/http/server.go:3210 +0x8e
    net/http.(*conn).serve(0xc00029e240, {0xc80a58, 0xc0002c4060})
    /usr/local/go/src/net/http/server.go:2092 +0x5d0
    created by net/http.(*Server).Serve in goroutine 7
    /usr/local/go/src/net/http/server.go:3360 +0x485

    Code used to reproduce a bug yang/library.yang

    module library {
    revision 2024-10-03;  
          list books {
          key book-id;
    
              leaf book-id {
              description "unique id for book";
                  type int32;
              }
    
              leaf title {
              description "title of the book";
                  type string;
              }
    }
    }

    main.go

    
    package main

import ( "github.com/freeconf/restconf" "github.com/freeconf/restconf/device" "github.com/freeconf/yang/fc" "github.com/freeconf/yang/node" "github.com/freeconf/yang/nodeutil" "github.com/freeconf/yang/source" )

type Book struct { BookId int Title string }

type Library struct { Books []*Book }

type Model struct { Library *Library }

func NewLibrary() Library { lib := &Library{ Books: []Book{}, } return lib }

func getByKey(n *nodeutil.Node, r node.ListRequest) (node.Node, error) { // node processing... return n.DoGetByKey(r) }

func main() { fc.DebugLog(true) ypath := source.Any(source.Path("yang"), restconf.InternalYPath) d := device.New(ypath) rootNode := &nodeutil.Node{ Object: NewLibrary(), OnGetByKey: getByKey, } d.Add("library", rootNode) restconf.NewServer(d) d.ApplyStartupConfigFile("./startup.json") select {} }

`startup.json`
```json
{
    "fc-restconf": {
        "web": {
            "port": ":8080"
        }
    },
    "library": {
        "books": [
            {
                "book-id": 0,
                "title": "Book nr 0"
            },
            {
                "book-id": 1,
                "title": "Book nr 1"
            }
        ]
    }
}

Additional informations go.mod content:

module keyhandlers

go 1.23.0

require (
    github.com/freeconf/restconf v0.0.0-20240627124255-9543e7933a6e // indirect
    github.com/freeconf/yang v0.0.0-20240126135339-ef92ddeb9f99 // indirect
)

The only way I found to avoid it, is to implement an explicit OnDeleteByKey handler too. After that requested DELETE request is performed with no issues.

package main

import (
    "github.com/freeconf/restconf"
    "github.com/freeconf/restconf/device"
    "github.com/freeconf/yang/fc"
    "github.com/freeconf/yang/node"
    "github.com/freeconf/yang/nodeutil"
    "github.com/freeconf/yang/source"
)

type Book struct {
    BookId int
    Title  string
}

type Library struct {
    Books []*Book
}

type Model struct {
    Library *Library
}

func NewLibrary() *Library {
    lib := &Library{
        Books: []*Book{},
    }
    return lib
}

func getByKey(n *nodeutil.Node, r node.ListRequest) (node.Node, error) {
    // node processing...
    return n.DoGetByKey(r)
}

func deleteByKey(n *nodeutil.Node, r node.ListRequest) error {
    return n.DoDeleteByKey(r)
}

func main() {
    fc.DebugLog(true)
    ypath := source.Any(source.Path("yang"), restconf.InternalYPath)
    d := device.New(ypath)
    rootNode := &nodeutil.Node{
        Object:        NewLibrary(),
        OnGetByKey:    getByKey,
        OnDeleteByKey: deleteByKey,
    }
    d.Add("library", rootNode)
    restconf.NewServer(d)
    d.ApplyStartupConfigFile("./startup.json")
    select {}
}