alexejk / go-xmlrpc

An XML-RPC Client for Go
https://alexejk.io/article/handling-xmlrpc-in-go/
MIT License
19 stars 7 forks source link

Failure to produce request with a []any as params #86

Closed burgesQ closed 5 months ago

burgesQ commented 6 months ago

I cannot successfully produce the following request payload:

<?xml version="1.0"?>
<methodCall>
  <methodName>di</methodName>
  <params>
    <param><value><string>abc</string></value></param>
    <param><value><string>def</string></value></param>
    <param><value><string>hij</string></value></param>
  </params>
</methodCall>
Attempt to use a struct ```golang if err := ec.Encode(os.Stdout, "di", struct{ Params []string }{Params: []string{"abc", "def", "hij"}}); err != nil { log.Fatalf("xmlrpc encode: %v", err) } ``` Produce an invalid payload ```xml di webconference listRooms verysecret ```
Attempt to use an array Produce a fatal ```golang if err := ec.Encode(os.Stdout, "di", []any{"abc", "def", "hij"}); err != nil { log.Fatalf("xmlrpc encode: %v", err) } --> dipanic: reflect: call of reflect.Value.NumField on slice Value goroutine 1 [running]: reflect.flag.mustBe(...) /home/master/repo/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.1.linux-amd64/src/reflect/value.go:233 reflect.Value.NumField({0x6a0300?, 0xc00018e000?, 0x497c2a?}) /home/master/repo/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.1.linux-amd64/src/reflect/value.go:2127 +0x85 alexejk.io/go-xmlrpc.(*StdEncoder).encodeArgs(0xc00008fe7f, {0x78b0a0, 0xc000098020}, {0x6a0300?, 0xc00018e000?}) /home/master/repo/go/pkg/mod/alexejk.io/go-xmlrpc@v0.5.2/encode.go:37 +0x105 alexejk.io/go-xmlrpc.(*StdEncoder).Encode(0xc00008fe7f, {0x78b0a0, 0xc000098020}, {0x706aa3?, 0x0?}, {0x6a0300, 0xc00018e000}) /home/master/repo/go/pkg/mod/alexejk.io/go-xmlrpc@v0.5.2/encode.go:24 +0xae main.main() /home/master/repo/sbc/src/common/api/gopi/test/xmlrpc.go:36 +0x16c ```
alexejk commented 6 months ago

Hi @burgesQ, I believe you misunderstand how this library should be used.

To make an XML-RPC call, you are not expected to use Encoder directly, but instead rely on the Client returned by NewClient method and using it's Client.Call to make a request to a method (di in your case, with a struct defining parameters). This is because I opted to use the Stdlib RPC mechanics behind the scenes.

Here is a test case showing how this can be done:

func TestClient_Github_86(t *testing.T) {
    ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        m := &struct {
            Name   string           `xml:"methodName"`
            Params []*ResponseParam `xml:"params>param"`
        }{}
        body, err := io.ReadAll(r.Body)
        require.NoError(t, err, "test server: read body")

        err = xml.Unmarshal(body, m)
        require.NoError(t, err, "test server: unmarshal body")

        require.Equal(t, "di", m.Name)
        require.Equal(t, 3, len(m.Params))

        expected := []string{"abc", "def", "hij"}
        for i, p := range m.Params {
            require.Equal(t, expected[i], *p.Value.String)
        }

        respBody := `<?xml version="1.0"?>
<methodResponse>
    <params>
        <param>
            <value><string>OK</string></value>
        </param>
        <param>
            <value><int>200</int></value>
        </param>
    </params>
</methodResponse>`
        _, _ = fmt.Fprintln(w, respBody)
    }))
    defer ts.Close()

    c, err := NewClient(ts.URL)
    require.NoError(t, err)
    require.NotNil(t, c)

    type DiRequest struct {
        First  string
        Second string
        Third  string
    }

    type DiResponse struct {
        Status string
        Code   int
    }

    req := &DiRequest{
        First:  "abc",
        Second: "def",
        Third:  "hij",
    }
    resp := &DiResponse{}

    err = c.Call("di", req, resp)
    require.NoError(t, err)
    require.Equal(t, "OK", resp.Status)
    require.Equal(t, 200, resp.Code)
}

Things to note in particular:

Hope this helps!

github-actions[bot] commented 5 months ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

burgesQ commented 5 months ago

Hope this helps!

Yes, a lot, thanks a ton ! I was finally able to request the legacy API. Using some anonymous struct seems to be the go-to (nice for code clarity). Sadly, composition seems to provoke struct in struct for the request :(

Request composition In the following example, `Req` and `ReqCompo` won't generate the same request (somehow expected, I believe ?) ```golang type ( Module struct { Module string } ReqCompo { Module Method string } Req { Module string Method string } ) ``` That's more than fine I can live with it :)

But I'm facing a new issue, dealing with the API response payload this time.

Most of the response can be decoded in simple struct which is way nicer:

type DiResponse struct {
    Out [][]any
}

--> &{Out:[[192.168.1.233 5060]]}
Request decoding array of struct Sadly, an array of anonymous won't work: ```golang type DiResponseArrayStruct struct { Out [][]stuct { Ip string Port int } } // --> reading body failed decoding array item at index 0: failed decoding array item at index 0: type 'struct { IP string; Port int }' cannot be assigned a value of type 'string' type DiResponseStruct struct { Out []stuct { Ip string Port int } } // --> reading body failed decoding array item at index 0: invalid field type: expected 'slice', got 'struct' ```

And in some cases - where I expect an array with an int, an array of int and a string aka [200 [1234] Server: Some Server (5.4.0-d7d378629f (x86_64/Linux)) ], attempt to use any of struct { Out []any } or struct { Out [][]any } failed ! That one is a bummer for me sadly :(

alexejk commented 5 months ago

@burgesQ could you please provide me the network caps for request and response (specifically the XML parts) so I can check why that is giving you troubles and give more guidance (or see if there is something that needs to be improvedin the lib)?

burgesQ commented 5 months ago

The one I can't work around at the moment is the following: I'm not able to decode that particular response payload:

<value>
  <array>
    <data>
      <value><i4>200</i4></value>
      <value><array><data><value>Some String</value></data></array></value>
      <value>Server: Sip Express Media Server (5.1.0 (x86_64/Linux)) calls: 0 active/0 total/0 connect/0 min</value>
    </data>
  </array>
</value>

Attempt to use an array of any array

type DiResponseArrayOfAnyArray struct {
    Out [][]any
}

type DiResponseAnyArray struct {
    Out []any
}

Result in the following error: reading body failed decoding array item at index 0: type '[]interface {}' cannot be assigned a value of type 'int'

A single array of interface result in reading body failed decoding array item at index 1: invalid field type: expected 'slice', got 'interface'

Attempt to use an array of named or anonymous struct also fail:

&struct {
        Out []struct {
            Code   int
            Out    []any
            Status string
        }
    }

Result in the following error: reading body failed decoding array item at index 0: type '[]struct { Code int; Out []interface {}; Status string }' cannot be assigned a value of type 'int'

alexejk commented 5 months ago

Hi @burgesQ, thank you for providing the sample response. This allowed me to trace it to a limitation in the library that I believe should now be fixed with 0.5.3 version.

Specifically, the issue was in handling of the nested arrays in decoder when arrays have mixed types (interface types effectively).

You can take a look at the sample test in https://github.com/alexejk/go-xmlrpc/blob/master/client_test.go#L71 that mimics the API response you've provided, but effectively you should define a single level []any response field, and then handle parsing / type-casting on your own, as it won't be possible to strongly type those responses further than that (mixed arrays by definition mean no strong typing). Some basic assertions could be seen in same test-case the https://github.com/alexejk/go-xmlrpc/blob/master/client_test.go#L145-L150

There are a few more tests that may provide additional inspiration in decoder itself (mainly related to multi-level nesting of mixed arrays).

Hope this helps!

burgesQ commented 5 months ago

Wonderful thanks a ton !