Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
burgesQ opened this issue Apr 2, 2024 · 7 comments · Fixed by #87
Closed

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

burgesQ opened this issue Apr 2, 2024 · 7 comments · Fixed by #87
Assignees
Labels
bug Something isn't working

Comments

@burgesQ
Copy link

burgesQ commented Apr 2, 2024

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
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

<methodCall>
  <methodName>di</methodName>
  <params>
    <param><value><array><data>
      <value><string>webconference</string></value>
      <value><string>listRooms</string></value>
      <value><string>verysecret</string></value>
    </data></array></value></param>
  </params>
</methodCall>
Attempt to use an array

Produce a fatal

if err := ec.Encode(os.Stdout, "di", []any{"abc", "def", "hij"}); err != nil {
		log.Fatalf("xmlrpc encode: %v", err)
	}
-->
<methodCall><methodName>di</methodName>panic: 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
Copy link
Owner

alexejk commented Apr 7, 2024

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:

  • Notice that DiRequest does not have the method name. This is because net/rpc Client provides you with Call(serviceMethod string, args any, reply any) error method that receives the method as a first parameter
  • DiRequest is encoded in a special way. You could use nested structs and similar in it, but the outer argument is a single struct as per Call` method definition
  • I chose to use a mock client here, but you can see that all parameters are validated on the handler side

Hope this helps!

@alexejk alexejk added question General guideline questions waiting for feedback Issue pending information/response from reporter labels Apr 7, 2024
@alexejk alexejk self-assigned this Apr 7, 2024
Copy link

github-actions bot commented May 8, 2024

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.

@github-actions github-actions bot added the Stale label May 8, 2024
@burgesQ
Copy link
Author

burgesQ commented May 13, 2024

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 ?)

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:

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 alexejk removed the Stale label May 13, 2024
@alexejk
Copy link
Owner

alexejk commented May 13, 2024

@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
Copy link
Author

burgesQ commented May 14, 2024

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 alexejk added bug Something isn't working question General guideline questions waiting for feedback Issue pending information/response from reporter and removed question General guideline questions waiting for feedback Issue pending information/response from reporter labels May 16, 2024
@alexejk
Copy link
Owner

alexejk commented May 16, 2024

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
Copy link
Author

burgesQ commented May 16, 2024

Wonderful thanks a ton !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants