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

python3-fastrpc and xmlrpc.client compatibility #33

Open
sejvlond opened this issue May 30, 2016 · 9 comments
Open

python3-fastrpc and xmlrpc.client compatibility #33

sejvlond opened this issue May 30, 2016 · 9 comments
Milestone

Comments

@sejvlond
Copy link

sejvlond commented May 30, 2016

python3-fastrpc (7.0.3)
xmlrpc.client.__version__ == '3.4'

fastrpc does not return tuple, when method name is not provided

In [6]: fastrpc.loads(fastrpc.dumps(("test",)))
Out[6]: ('test', None)

In [7]: xmlrpc.client.loads(xmlrpc.client.dumps(("test",)))
Out[7]: (('test',), None)

it is ok when method name is set

In [8]: fastrpc.loads(fastrpc.dumps(("test",), "xxx"))
Out[8]: (('test',), 'xxx')

In [9]: xmlrpc.client.loads(xmlrpc.client.dumps(("test",), "xxx"))
Out[9]: (('test',), 'xxx')

fastrpc raises RuntimeError: Parser error:< Empty document > but only on strings more than 3 chars long:

In [10]: fastrpc.loads("asda")
RuntimeError: Parser error:< Empty document >

Otherwise no error occures

In [12]: fastrpc.loads("asd")
Out[12]: (None, None)
In [13]: xmlrpc.client.loads("asd")
ExpatError: syntax error: line 1, column 0

So there is no chance how to recognize parse error, or validly returned None with no method name..

# OK
In [52]: fastrpc.loads(fastrpc.dumps((None,), "xxx"))
Out[52]: ((None,), 'xxx')

# Validly returned None
In [53]: fastrpc.loads(fastrpc.dumps((None,), None))
Out[53]: (None, None)

# Invalid, but same as valid None
In [63]: fastrpc.loads("asd")
Out[63]: (None, None)

# OK
In [58]: xmlrpc.client.loads(xmlrpc.client.dumps((None,), "xxx", allow_none=True))
Out[58]: ((None,), 'xxx')

# OK
In [57]: xmlrpc.client.loads(xmlrpc.client.dumps((None,), None, allow_none=True))
Out[57]: ((None,), None)

# OK
In [64]: xmlrpc.client.loads("asd")
ExpatError: syntax error: line 1, column 0

Loading xmlrpc.client.Fault with fastrpc fails to set faultCode - that means it is not possible to use fastrpc on xml documents generated with xmlrpc lib

In [49]: fastrpc.loads(xmlrpc.client.dumps(xmlrpc.client.Fault(504, "Err")))
Fault: <fastrpc.Fault: 0, Err>
In [51]: fastrpc.loads(fastrpc.dumps(fastrpc.Fault(504, "Err")))
Fault: <fastrpc.Fault: 504, Err>
@volca02
Copy link
Contributor

volca02 commented May 31, 2016

The way loads handles data is dependent on fastRPC library itself, which is trying to be smart, deciding whether it should use Binary or XML based unmarshaller by looking at the stream length and first 2 bytes.

You can craft arbitary length sequences that will pass as valid data, for example <sdfadsfasdfa

I don't currently see a way to enforce Binary unmarshaller to force the validation, I'll see what I can do here.

@volca02 volca02 added this to the Fastrpc 8 milestone May 31, 2016
@volca02
Copy link
Contributor

volca02 commented Jun 1, 2016

I found the reason the faultCode is sometimes unset - there is no guarantee the faultCode will be the first element in the xml stream:

<?xml version='1.0'?>\n<methodResponse>\n<fault>\n<value><struct>\n<member>\n<name>faultCode</name>\n<value><int>504</int></value>\n</member>\n<member>\n<name>fau ltString</name>\n<value><string>Err</string>...

or

<?xml version='1.0'?>\n<methodResponse>\n<fault>\n<value><struct>\n<member>\n<name>faultString</name>\n<value><string>Err</string></value>\n</member>\n<member>\n< name>faultCode</name>\n<value><int>504</int>...

The code that parses the fault is not able to handle the faultCode if it comes as a second value.

@sejvlond
Copy link
Author

sejvlond commented Jun 1, 2016

faultCode

ok, but it's a bug, right? and should be fixed in major 8 version?

fastRPC library itself, which is trying to be smart

would be nice to have some parameter in fastrpc.loads where I can specify content-type, for example:
fastrpc.loads(data, use_binary=True/False/None) where None should be default and it means, try to load it at your discretion (backward compatible) and True and False means use fastrpc or use xmlrpc. Because I know, what data I've got from content-type header.

@volca02
Copy link
Contributor

volca02 commented Jun 1, 2016

Yep, it's a bug. I'll fix it in frpc8.

I'll look into the kwargs for loads, it seems possible to do.

@volca02
Copy link
Contributor

volca02 commented Jun 2, 2016

We decided not to change the behavior of loads (tuple/non-tuple parameters) as it would likely break existing usage and would be hard to fix. We may implement an opt-in change later on, but we don't like the idea of postponing the frpc8 release to implement this.

The useBinary for loads will probably make it into the release.

@volca02
Copy link
Contributor

volca02 commented Jun 2, 2016

useBinary in loads was implemented in 01f566c

@sejvlond
Copy link
Author

sejvlond commented Jun 2, 2016

We decided not to change the behavior of loads (tuple/non-tuple parameters)

and the behavior is: When method is not set, returns non-tuple, when method is set, returns tuple?

@volca02
Copy link
Contributor

volca02 commented Jun 2, 2016

Yes. There is unknown amount of code that relies on the current behavior.

@sejvlond
Copy link
Author

sejvlond commented Jun 2, 2016

ok, I will wrap this for myself as well

volca02 pushed a commit that referenced this issue Jun 15, 2016
commit 20baaf8
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Tue Jun 14 08:54:55 2016 +0200

    forgotten python changelog

commit ed1661d
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Mon Jun 13 10:52:08 2016 +0200

     #! v8.0.0-rc2

commit 7dd55dd
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed Jun 8 10:38:23 2016 +0200

    Fixing format string warnings.

commit 5772fa9
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed Jun 8 10:25:58 2016 +0200

    Transitioned to explicit {Error_t,...}::format call for va_args style exceptions. Fixes possible nasty implicit casting errors.

commit 39b99e8
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed Jun 8 07:59:18 2016 +0200

    Don't ever return a nullptr as reference.
    Throws Fault_t as it should when unmarshalled data
    is null and value is requested by reference.

commit 39c3637
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Thu Jun 2 15:12:27 2016 +0200

    8.0.0-rc1

commit 1a9a656
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Thu Jun 2 14:57:33 2016 +0200

    fixed lib versioning for frpc8

commit 580081f
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Thu Jun 2 14:39:53 2016 +0200

    rolling back the 8.0 versioning, will be re-added in rc1 release

commit f0192c8
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Thu Jun 2 13:58:32 2016 +0200

    missing include added

commit 24d454d
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Thu Jun 2 13:11:14 2016 +0200

    Missing protocol test added

commit 01f566c
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Thu Jun 2 10:14:41 2016 +0200

    Allow kwargs in loads. Added new optional parameter useBinary (True, False, None) that specifies if the loads should enforce binary decoding.

commit 569215d
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed Jun 1 13:00:51 2016 +0200

    Fix for #33 - don't depend on xml node ordering when building a fault object.

commit af53fdb
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed Jun 1 12:16:19 2016 +0200

    Cleanup, formatting

commit 5b5f729
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Thu May 26 13:08:52 2016 +0200

    Fix for #24. Removed the enclosing, one element array from the result structure

commit 48965f7
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Thu May 26 09:04:12 2016 +0200

    fixes to the python3 version of the python bindings

commit 4ab6e22
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Thu May 26 08:43:36 2016 +0200

    Switched the nativeBoolean to be true by default, meaning fastrpc will implicitly use native python bool as a default

commit 4cfe8e4
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed May 25 10:18:13 2016 +0200

    renamed configure.in to configure.ac

commit 0fb7330
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed May 25 10:17:42 2016 +0200

    added tests for base64 and protocol changes, runnable via 'make check'

commit 09e22e7
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Tue May 24 14:48:08 2016 +0200

    #28 finished the changes needed to implement the new protocol version

commit 4169e9c
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Fri May 20 11:34:02 2016 +0200

    Implemented zigzag int decoding. Rised the maximal protocol major version handled by decoder

commit b3a39e7
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed May 18 14:49:06 2016 +0200

    pc file rename fix

commit f1aed03
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed May 18 14:10:06 2016 +0200

    Revert "versioned pc file"

    This reverts commit 570eb77.

commit de15ffa
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed May 18 14:09:59 2016 +0200

    Revert "versioned dev package"

    This reverts commit 5048b88.

commit d306624
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed May 18 14:02:54 2016 +0200

    removed todo, that part was already implemented

commit 7524567
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed May 18 14:02:31 2016 +0200

    Missed this rename

commit 5a15326
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed May 18 13:24:44 2016 +0200

    Untested: implementation of custom headers in python bindings

commit 6467945
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed May 18 13:23:58 2016 +0200

    adding zigzagDecode routine (to be used by int decoder for protocol version 3)

commit 0f9af39
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed May 18 13:23:12 2016 +0200

    Typo fix

commit 3b20557
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed May 18 13:22:44 2016 +0200

    Preparations for protocol change. Datetime packing/unpacking implemented (untested)

commit 763eda7
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed May 18 13:02:13 2016 +0200

    INSTALL file update

commit ef4b506
Merge: 7e57e59 aa1649d
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Tue May 17 09:56:47 2016 +0200

    Merge branch 'master' into fastrpc8

commit 7e57e59
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Tue May 10 15:17:08 2016 +0200

    Fixes for new gcc narrowing checks

commit daebc86
Merge: 36ade14 6aaefcc
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Wed May 4 09:25:08 2016 +0200

    Merge branch 'master' into fastrpc8

commit 36ade14
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Mon May 2 10:27:40 2016 +0200

    Manual reimplementation of d4392cb, 920f2f2

commit 0c884c2
Author: Ondrej Kocian <ondrej.kocian@firma.seznam.cz>
Date:   Tue Jun 24 13:35:37 2014 +0200

    library name link fixed

commit 6322845
Author: Ondrej Kocian <ondrej.kocian@firma.seznam.cz>
Date:   Thu May 15 11:33:42 2014 +0200

    missing include sstream

commit 5048b88
Author: Ondrej Kocian <ondrej.kocian@firma.seznam.cz>
Date:   Mon May 26 11:30:28 2014 +0200

    versioned dev package

commit c0be579
Author: Ondrej Kocian <ondrej.kocian@firma.seznam.cz>
Date:   Mon May 26 15:51:26 2014 +0200

    Vcs info

commit 570eb77
Author: Filip Volejnik <filip.volejnik@firma.seznam.cz>
Date:   Mon May 2 09:47:56 2016 +0200

    versioned pc file

commit 3f7aea2
Author: Ondrej Kocian <ondrej.kocian@firma.seznam.cz>
Date:   Sun May 4 22:16:27 2014 +0200

    WIP: merged the custom headers code into master branch, preparations for version 8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants