sonic-net / sonic-gnmi

SONiC gNMI server and gNOI repo
Other
23 stars 52 forks source link

Fix a panic by gnmi subscribe to DB change with invalid xpath #309

Closed wumiaont closed 1 week ago

wumiaont commented 1 month ago

Why I did it

It is found that if I run gnmi subscribe to db change with an invalid xpath, gnmi server has a panic because of nil pointer dereference.

Test is against multi-asic chassis. In telemtery test test_on_change_updates is did not handle the multi-asic properly which sends out gnmi subscribe like this: python /root/gnxi/gnmi_cli_py/py_gnmicli.py -g -t 10.250.6.233 -p 50052 -m subscribe -x NEIGH_STATE_TABLE -xt STATE_DB -o ndastreamingservertest --subscribe_mode 0 --submode 1 --interval 0 --update_count 2 --create_connections 1

gnmi container restarts with a panic.

Correct CLI commands should be: python /root/gnxi/gnmi_cli_py/py_gnmicli.py -g -t 10.250.6.233 -p 50052 -m subscribe -x NEIGH_STATE_TABLE/asic0 -xt STATE_DB -o ndastreamingservertest --subscribe_mode 0 --submode 1 --interval 0 --update_count 2 --create_connections 1

How I did it

Fix the deference of nil pointer with this scenario.

How to verify it

With modified code, the above CLI will not cause gnmi panic. The connection is properly closed.

Which release branch to backport (provide reason below if selected)

zbud-msft commented 3 weeks ago

Introduced by https://github.com/sonic-net/sonic-gnmi/pull/267

arlakshm commented 2 weeks ago

/Azp run sonic-net.sonic-gnmi

azure-pipelines[bot] commented 2 weeks ago
Azure Pipelines successfully started running 1 pipeline(s).