Client fails to parse large, complex response

[agnat]

Instead of an response object I just get null.

I gisted the response XML here: https://raw.github.com/gist/1852071/c6195cd12be6d77be16b68a26e183ae9a76b9195/gistfile1.txt

Would be great if you could take a look.

Cheers,

agnat

[baalexander]

Thanks @agnat for the XML.

Do you happen to know if the response is coming in as a chunked response? Or one big XML. You can check by adding a console.log or breakpoint in client.js to see if it gets called more than once.

[agnat]

Thanks for the quick response.

Yep, the xml response is chunky. I already tried ^HEAD instead of the npm package, because there seemed to be related issues. Same result.

[agnat]

Here is a little interoperability test, demonstrating the issue:

https://gist.github.com/1856367

It spawns a python xmlrpc server and pumps arguments through it. The results are compared with the input. The implementation is a bit sloppy, but a functional test like this would make a good extension to the existing test suite.

The test requires isaacs slide and the xmlrpc module ... obviously ...

[baalexander]

I've added your XML as a test case locally and have been trying to debug the issue. I'm getting the null response you were even when the data isn't chunked.

I don't have a definite idea what the issue is yet, but my current thought is the deserializeParams callback is being fired prematurely. My best guess right now is a callback issue when deserializing array params.

I'm hoping tomorrow I can take another look and try to at least remove as much of the XML as possible yet still get the error. This should help stepping through the program.

[agnat]

Thanks for the update. In the meantime I've rewritten the parser/unmarshalling part. It is loosely modeled after pythons xmlrpclib. It passes all tests including the nasty one in question. It's non-strict like yours. It uses a stacks (actually two stacks) to reconstruct the data structure. The main advantage is that it is much easier to understand what is going on. It still has a few loose ends but you might want to take a look:

https://github.com/agnat/node-xmlrpc/blob/master/lib/unmarshall.js

What do you think?

On 18.02.2012, at 23:52, Brandon Alexander wrote:

I've added your XML as a test case locally and have been trying to debug the issue. I'm getting the null response you were even when the data isn't chunked.

I don't have a definite idea what the issue is yet, but my current thought is the deserializeParams callback is being fired prematurely. My best guess right now is a callback issue when deserializing array params.

I'm hoping tomorrow I can take another look and try to at least remove as much of the XML as possible yet still get the error. This should help stepping through the program.

---

Reply to this email directly or view it on GitHub:
#42 (comment)

[baalexander]

You have made my day @agnat! I think unmarshalling is a much better solution than the current XML-RPC parsing algorithm.

Here's a few of my thoughts on the file:

• It can be renamed to the current xmlrpc-parser.js (and that will go away).
• The DISPATCH methods can be removed and the end functions called directly in a switch case in the Unmarshaller.end().
• Instead of taking a callback in the constructor, it can emit events for 'error' and 'end'. And/or adding functions parseMethodCall and parseMethodResponse that would take the callback.
• Sax.js's stream parser may allow for a better way to handle opentags and endtags without needing to use apply(). Like saxStream.on('opentag', this.start).
• Support method calls and use the unmarshaller in server.js.
• Date conversion can use the existing date-formatter.js
• Not sure what the parse_pi function is for.
• Some minor formatting (removing semicolons, removing the .prototype = {}, etc.)

I would be happy to make these changes or you can make some of them and send a pull request in.

[agnat]

Great you like it. I think it's easier if you do the rest. Most of it is a matter of style and personal preference and you know best how you like it. And, to be honest, I'd like to return to the task at hand...

• IIRC the only thing missing for the server side is method name handling.
• Regarding the date conversion: I think the regex I found covers a few more cases, but I can not proof it.
• Concerning the PI: The python implementation uses it to determine the encoding (which is nice to have but probably YAGNI)

I also have a few small feature/change requests to get this thing fit for production:

• I'd love to see user-selectable XML parsers, similar to the python implementaion. Isaacs parser is fine, but for production, in a potentially hostile environment, I'd prefer to use expat or something even if it does mean compiling stuff.
• Handling of large integers (I8): The old code used parseFloat(), which is just not correct. I'd like to have an option to keep the string values.
• The test suite needs improvement. For a protocol implementation much more scrutiny is in order. I'd like to see a large set of fixtures and at least one functional test that tests things against a different implementation (like the python thing in the comment above).

I think although it is a bit old fashioned xmlrpc still is a very important protocol and it would be great to get this thing ship-shape.

[agnat]

Fixed in 74bf8f2. The new deserializer correctly handles arbitrarily nested data structures. One thing still missing is a good test. There is a small grinder test but it didn't catch this issue. Maybe we should just add the large XML above as a test case.

What do you think?

[baalexander]

I added a test case with your XML method response. I wrote the output from the unmarshaller to a file as JSON. Really, though, the JSON should come from something like Python's XML-RPC module for actual verification.

That said, the deepEqual is failing when comparing the JSON with the unmarshaller's results (I commented out the comparison for now). It's probably something simple I missed, I can take a deeper look tomorrow evening.

[baalexander]

I believe this is fixed with PR #81. If any issues still, please comment or file another issue.