Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Pawel-Guzik opened this issue Oct 3, 2024 · 0 comments

Comments

@Pawel-Guzik
Copy link

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

{
    "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 {}
}
@Pawel-Guzik Pawel-Guzik changed the title Explicit implementation of 'OnGetByKey' handler, causes panic during delete node operation if 'OnDeleteByKey' is not explicit implemented Explicit implementation of 'OnGetByKey' handler, causes panic during delete node operation if 'OnDeleteByKey' is not explicitly implemented Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant