Fix crash on ListPathRequest with malformed prefix

[rodrigopv]

When ListPathRequest is done by a gRPC client including a malformed prefix,
the server would crash with an invalid memory address reference.

This commit fixes the crash by checking whether the parseCIDR method returned an error.

[rodrigopv]

Added tests to cover the cases I found + other inputs that can make it to this method through gRPC (arbitrary text). Also fixed an error in my first approach 🚀

[rodrigopv]

Just as a note, same as my other PR, tests are passing when I run them locally, so unless there's a style change request this should be good to merge.

Thanks!

[fujita]

Why not just return an error?

[rodrigopv]

I was going for a conservative approach as the current version was doing prefix = NewIPAddrPrefix(0, "") on certain code paths (but maybe for different reasons). Can't remember whether I tried throwing an error on this one though.

I'll try an approach that throws an error instead when an invalid CIDR and I'll post updates here 👌

[rodrigopv]

Ok now it passes all the tests, yet it is crashing again when a malformed prefix is passed because of a nil pointer somewhere else:

[signal SIGSEGV: segmentation violation code=0x1 addr=0x70 pc=0x1733c4b]

goroutine 43 [running]:
github.com/osrg/gobgp/v3/internal/pkg/table.(*Table).tableKey(0x20?, {0x0?, 0x0?})
	/Users/rodrigo.pena/gobgp/internal/pkg/table/table.go:407 +0x48b
github.com/osrg/gobgp/v3/internal/pkg/table.(*Table).GetDestination(...)
	/Users/rodrigo.pena/gobgp/internal/pkg/table/table.go:206
github.com/osrg/gobgp/v3/internal/pkg/table.(*Table).Select.func1({0xc00003a0d8?, 0xc000012048?})
	/Users/rodrigo.pena/gobgp/internal/pkg/table/table.go:477 +0xac
github.com/osrg/gobgp/v3/internal/pkg/table.(*Table).Select(0xc0001c3c80, {0xc0000b8e10, 0x1, 0x27b5d28?})
	/Users/rodrigo.pena/gobgp/internal/pkg/table/table.go:525 +0x1296
github.com/osrg/gobgp/v3/pkg/server.(*BgpServer).getRib.func1()
	/Users/rodrigo.pena/gobgp/pkg/server/server.go:2542 +0x1cd
github.com/osrg/gobgp/v3/pkg/server.(*BgpServer).handleMGMTOp(0x17f9be0?, 0xc000010108)
	/Users/rodrigo.pena/gobgp/pkg/server/server.go:273 +0x89
github.com/osrg/gobgp/v3/pkg/server.(*BgpServer).Serve(0xc0002dc000)
	/Users/rodrigo.pena/gobgp/pkg/server/server.go:460 +0x5eb
created by main.main
	/Users/rodrigo.pena/gobgp/cmd/gobgpd/main.go:199 +0x145c
exit status 2

I'll dig deeper and propose an approach to handle this malformed input gracefully

[fujita]

Ok now it passes all the tests, yet it is crashing again when a malformed prefix is passed because of a nil pointer somewhere else:

[signal SIGSEGV: segmentation violation code=0x1 addr=0x70 pc=0x1733c4b]

goroutine 43 [running]:
github.com/osrg/gobgp/v3/internal/pkg/table.(*Table).tableKey(0x20?, {0x0?, 0x0?})
	/Users/rodrigo.pena/gobgp/internal/pkg/table/table.go:407 +0x48b
github.com/osrg/gobgp/v3/internal/pkg/table.(*Table).GetDestination(...)
	/Users/rodrigo.pena/gobgp/internal/pkg/table/table.go:206
github.com/osrg/gobgp/v3/internal/pkg/table.(*Table).Select.func1({0xc00003a0d8?, 0xc000012048?})
	/Users/rodrigo.pena/gobgp/internal/pkg/table/table.go:477 +0xac
github.com/osrg/gobgp/v3/internal/pkg/table.(*Table).Select(0xc0001c3c80, {0xc0000b8e10, 0x1, 0x27b5d28?})
	/Users/rodrigo.pena/gobgp/internal/pkg/table/table.go:525 +0x1296
github.com/osrg/gobgp/v3/pkg/server.(*BgpServer).getRib.func1()
	/Users/rodrigo.pena/gobgp/pkg/server/server.go:2542 +0x1cd
github.com/osrg/gobgp/v3/pkg/server.(*BgpServer).handleMGMTOp(0x17f9be0?, 0xc000010108)
	/Users/rodrigo.pena/gobgp/pkg/server/server.go:273 +0x89
github.com/osrg/gobgp/v3/pkg/server.(*BgpServer).Serve(0xc0002dc000)
	/Users/rodrigo.pena/gobgp/pkg/server/server.go:460 +0x5eb
created by main.main
	/Users/rodrigo.pena/gobgp/cmd/gobgpd/main.go:199 +0x145c
exit status 2

I'll dig deeper and propose an approach to handle this malformed input gracefully

Unit test failed? If so, needs to fix the unit test to handle an error properly.

[rodrigopv]

Now I implemented error handling on Table.Select() and add an additional unit test resembling the crash I posted in the last comment.

CI passes now, although we have the random race fail in ci/unit, but after retrying, it should be good 🤟.

Also, I did end-to-end testing by passing malformed prefixes via gRPC and gobgpd no longer crashes

[fujita]

Thanks!