CA-291197: Toolstack no longer ignores junk at the end of JSON-RPC re…

[YarsinCitrix]

…sponses from pvsproxy

This is resulted by CA-236855, in that the read strategy is changed to assume all good data will come from PVSproxy side. However, in testing, it is found a bug in PVSproxy, which was not fatal locally, but resulted issue in overall scenario. It is decided to enhance the solution a little bit based on previous change.

The updates as to integrate some part in the old byte by byte read, where a function is used to prescreen the data read from PVSproxy, and tailing junk is discarded, so the only json part is further processed. Here we most copied the original function, but added:

1. to get input chars from a string;
2. to tolerant heading spaces/junk (in screening);

Signed-off-by: YarsinCitrix yarsin.he@citrix.com

[YarsinCitrix]

I was trying to alter a little bit Yojson to get just one json out and ignores tailing junk, and then pass it to jsonrpc. The Yojson part could be done, as most of the related functions are exposed in read.mli. However, functions like json_to_rpc in jsonrpc is not exposed, so we cannot create an updated jsonrpc module to adopt the json out of yojson from the input string. The other way left is just use the updated Yojson to get the json, from the initial output string, and then convert the json back to string, and pass it to jsonrpc as normal. This is considered a little bit complex and less efficient, after discussion with Yang. So we are trying to start with integrate the original code.

[lindig]

Just some general remarks:

• I think a better name for retrieve_json would be strip or strip_json
• JSON is UTF8 encoded. Can we be sure that the manual parsing of JSON implemented in retrieve_json does not trip over that? I suspect we can because bytes in UTF8 that belong to a sequence have their highest bit set and thus can't contain characters like " or { that the scanner is looking for.
• I still would prefer a solution that does not try to implement a JSON parser and make both sides exchange proper JSON that does not require special massaging.

[edwintorok]

I agree with @lindig last point above, this sounds like a bug that should be fixed in pvsproxy and not here.

[robhoes]

It is also being fixed in pvsproxy, but we should be more robust in xcp-networkd as well. And we were robust in this way until a recent patch, which is another reason to revert to this behaviour for compatibility.

[jonludlam]

we kind-of have to fix this in xcp-networkd as well as pvsproxy because pvsproxy won't be upgraded at the same time as xcp-networkd (pvsproxy is delivered separately). Ideally I'd like to remove this in the next release.

[robhoes]

Meanwhile, the pvsproxy has been fixed to not have junk at the end of the message anymore, so the problem is now less urgent. However, we still want to fix it here as well (as hinted at above), but there is no need to make a temporary fix first. @jonludlam says that an update to ocaml-rpc to address this would not be too hard to do, and recommends doing that instead and closing this PR. So let's do that.