ironthree / dxr

Declarative XML-RPC in Rust
Apache License 2.0
17 stars 8 forks source link

Support for string values with no explicit type tag around them #19

Closed jobafr closed 9 months ago

jobafr commented 9 months ago

Hi,

I'm using this crate's server via ros-core-rs, and I'm trying to use rosbag as a client. rosbag uses the xmlrpc++ library to speak the xmlrpc protocol.

I was getting weird errors, and tracked it down to this TCP stream (see below) between rosbag (client, xmlrpc++) and ros-core-rs (server, dxr). It seems that rosbag omits the <string> type annotation around the values (/play_1707484132677982521, /tcp_keepalive). I looked into the standard, section "Scalar \<value>s", and it says "If no type is indicated, the type is string". So it seems rosbag/xmlrpc++ are actually behaving in a standard compliant way here. However, dxr is unable to parse this.

I'm not sure what the best way to fix this would be. Is serde flexible enough to handle this via some clever parameters to the derive macro for Deserialize, or would one have to implement Deserialize manually for Value to support this?

Here's the TCP dump (HTTP request and response) of the interaction:

POST / HTTP/1.1
User-Agent: XMLRPC++ 0.7
Host: localhost:11311
Content-Type: text/xml
Content-length: 201

<?xml version="1.0"?>
<methodCall><methodName>hasParam</methodName>
<params><param><value>/play_1707484132677982521</value></param><param><value>/tcp_keepalive</value></param></params></methodCall>
HTTP/1.1 200 OK
content-type: text/xml
content-length: 390
date: Fri, 09 Feb 2024 13:08:52 GMT

<methodResponse><fault><value><struct><member><name>faultCode</name><value><i4>400</i4></value></member><member><name>faultString</name><value><string>Failed to parse XML data: unknown variant `/play_1707484132677982521`, expected one of `i4`, `boolean`, `string`, `double`, `dateTime.iso8601`, `base64`, `struct`, `array`</string></value></member></struct></value></fault></methodResponse>
decathorpe commented 9 months ago

This has been fixed since dxr 0.6.0-beta.1: https://github.com/ironthree/dxr/releases/tag/0.6.0-beta.1

I would recommend that ros-core-rs update its dxr dependency. I'm not sure if I can backport the fixes for this to the v0.5 branch without breaking the public API.

jobafr commented 9 months ago

Thank you for looking into this. I can confirm this works when updating the dependency and making the appropriate API adjustments. I'll make a PR to ros-core-rs.