sony / nmos-cpp

An NMOS (Networked Media Open Specifications) Registry and Node in C++ (IS-04, IS-05)
Apache License 2.0
144 stars 80 forks source link

Unexpected parse success of "transport_params": [{"..."}, {}] #70

Closed billt-hlit closed 4 years ago

billt-hlit commented 5 years ago

Hi, I was testing some changes and accidentally ran a command like

curl -H 'Content-Type: application/json' -H 'Accept: application/json' -X PATCH http://d13:3215/x-nmos/connection/v1.0/single/senders/b0ec9bcf-d46d-5539-adc5-421bd4757166/staged -d '{"transport_params": [{ "10.10.164.211" }, {}], "activation": {"mode": "activate_immediate"}}'

not realizing I forgot the key that went with the value, and then not understanding why my intended change wasn't being performed by the node despite the successful response from it:

{"activation":{"activation_time":"1556732522:76546184","mode":"activate_immediate","requested_time":null},"master_enable":false,"receiver_id":null,"transport_params":[{"destination_ip":"auto","destination_port":"auto","rtp_enabled":true,"source_ip":"auto","source_port":"auto"},{"destination_ip":"auto","destination_port":"auto","rtp_enabled":true,"source_ip":"auto","source_port":"auto"}]}

The messages logged by the node didn't indicate anything wrong, either

2019-05-01 17:47:23.851: info: 140737177954048: Operation requested for single sender: b0ec9bcf-d46d-5539-adc5-421bd4757166
2019-05-01 17:47:23.872: more info: 140737177954048: Validating staged transport parameters against constraints
2019-05-01 17:47:23.875: info: 140737298425600: Processing immediate activation for sender: b0ec9bcf-d46d-5539-adc5-421bd4757166
2019-05-01 17:47:23.876: more info: 140736892602112: Sending response

I don't believe this should be valid JSON. Is this a parser problem?

I checked this against 8d3f05a635026aa6f37ecce989a9df13b9b22acd using my usual build environment -- cross-compiling from Ubuntu Linux to home-grown Linux using gcc 6.2 and libc 2.17.

garethsb commented 5 years ago

I agree, looks like a flaw in the cpprestsdk json parser. I'll investigate.

garethsb commented 5 years ago

Yes, it was a C++ REST SDK issue: fix at https://github.com/Microsoft/cpprestsdk/pull/1130

garethsb commented 5 years ago

Necessary patch merged to cpprestsdk master. I imagine there'll be a release along soon...

garethsb commented 5 years ago

C++ REST SDK v2.10.14 is tagged. I haven't tested the release yet.

garethsb commented 5 years ago

Now tested, and I can't quite recommend it unconditionally due to https://github.com/microsoft/cpprestsdk/pull/1202.

garethsb commented 4 years ago

C++ REST SDK v2.10.14 is released.