json-rpc search.performSearch results in 'maximum recursion depth exceeded' Error

[Marco Antonio Islas Cruz]

When trying to use json-rpc search this it results in this error:

{    u'error': {    u'code': -32603,
                    u'message': u'maximum recursion depth exceeded',
                    u'name': u'JSONRPCError'},
     u'id': None,
     u'result': None}

Data used in the query (althought it ends with the error if the search returns something):

{   'method': 'search.performSearch', 'params': ['proxy']}

As I've said, this error appears if something is found by the search and makes the json-rpc call unusable.

[Marco Antonio Islas Cruz]

The problem here seems to be the TracRpcJSONEncoder json encoder, it helps by adding a clue of binary and datetime types by returning a jsonclass list with the first element being the clue and the second a string representation of the element. But when TracRpcJSONEncoder doesn't know how to handle data it passes it to JSONEncoder which for some reason is doing a recursión.

My proposed fix is, instead of just calling to JSONEncoder, catch if there is any error, if there is just return a representation of the element (repr(obj)).

        def default(self, obj):
            if isinstance(obj, datetime.datetime):
                # http://www.ietf.org/rfc/rfc3339.txt
                return {'__jsonclass__': ["datetime",
                                obj.strftime('%Y-%m-%dT%H:%M:%S')]}
            elif isinstance(obj, Binary):
                return {'__jsonclass__': ["binary",
                                obj.data.encode("base64")]}
            elif obj is empty:
                return ''
            else:
                try:
                    return json.JSONEncoder(self, obj)
                except:
                    return repr(obj)

I'm currently using Trac 1.0 and I'm not sure if this bug is present in previous versions but it seems that json can't encode a <fragment> object which seems to be part of the result to be returned to the user.

[Olemis Lang]

Replying to markuzmx:

The problem here seems to be the TracRpcJSONEncoder json encoder, it helps by adding a clue of binary and datetime types by returning a jsonclass list with the first element being the clue and the second a string representation of the element. But when TracRpcJSONEncoder doesn't know how to handle data it passes it to JSONEncoder which for some reason is doing a recursión.

Could you please mention Python version ? First thing needed is to identify whether this is a bug related to XmlRpcPlugin or otherwise json module (or equivalent) .

[anonymous]

Replying to olemis:

Could you please mention Python version ? First thing needed is to identify whether this is a bug related to XmlRpcPlugin or otherwise json module (or equivalent) .

I'm running Python 2.6.5

[osimons]

Oh, so some part of the search implementation returns items of type Fragment that we don't handle correctly? That isn't surprising, but what versions of Trac this affects is hard to say without testing. The trac.search backend is a terrible implementation from an RPC standpoint as it entangles the search results with its representation by returning HTML fragments.

Testing RPC search is currently not done. There should be a tracrpc/tests/search.py testing XML-RPC and JSON-RPC implementations of searching to make sure it works...

markuzmx, if you can be tempted to contribute a patch I've written an RPC development guide that should get you running tests in no time:

​https://www.coderesort.com/u/simon/blog/2010/09/23/tracrpc_development

[Marco Antonio Islas Cruz]

Of course, I'll take care of the tests ;-)

[Jun Omae]

Replying to osimons:

Oh, so some part of the search implementation returns items of type Fragment that we don't handle correctly?

Reproduced! And I got SEGV on Python 2.4.3 with simplejson 2.0.9....

TicketModule.get_search_results() returns Fragment (or maybe LazyProxy from tag_?) instances as title. Also, json_rpc.py don't handle them. See ​http://trac.edgewall.org/browser/branches/0.12-stable/trac/ticket/web_ui.py?rev=10639&marks=214-218#L186.

The _normalize_xml_output in xml_rpc.py handles correctly Fragment instances. See xmlrpcplugin/trunk/tracrpc/xml_rpc.py@9360#L178​.

I created the patch, attachment:ticket10786-xmlrpc-r12546.diff​.

[Olemis Lang]

Replying to jun66j5:

Replying to osimons:

Oh, so some part of the search implementation returns items of type Fragment that we don't handle correctly?

Reproduced! And I got SEGV on Python 2.4.3 with simplejson 2.0.9....

TicketModule.get_search_results() returns Fragment (or maybe LazyProxy from tag_?) instances as title. Also, json_rpc.py don't handle them. See ​http://trac.edgewall.org/browser/branches/0.12-stable/trac/ticket/web_ui.py?rev=10639&marks=214-218#L186.

The _normalize_xml_output in xml_rpc.py handles correctly Fragment instances. See xmlrpcplugin/trunk/tracrpc/xml_rpc.py@9360#L178​.

I created the patch, attachment:ticket10786-xmlrpc-r12546.diff​.

I'll try to review this in the next few days and attempt to write some test cases for search RPC handler (if possible) .

[Jun Omae]

Replying to olemis:

I'll try to review this in the next few days and attempt to write some test cases for search RPC handler (if possible) .

Thanks, Olemis! I added minimum unit tests for json-rpc with fragment and search, ticket10786-with-tests-xmlrpc-r12546.diff​. Could you please review the latest patch?

[Olemis Lang]

Replying to jun66j5:

Replying to olemis:

I'll try to review this in the next few days and attempt to write some test cases for search RPC handler (if possible) .

Thanks, Olemis! I added minimum unit tests for json-rpc with fragment and search, ticket10786-with-tests-xmlrpc-r12546.diff​. Could you please review the latest patch?

Trac version	Results
0.12	ok
1.0	​http://pastebin.com/wKqeJivW
trunk	​http://pastebin.com/yEjMttXH

[Jun Omae]

Replying to olemis:

|| trunk || http://pastebin.com/yEjMttXH ||

The failure in test_getActions brings the other two failures. Also, the failure happens with xmlrpcplugin/trunk-r12546, too. Then, that is another issue.

Anyway, I made test_fragment_in_search more robust. See ​https://github.com/jun66j5/xmlrpcplugin/compare/trunk...ticket10786 (​diff).

I'll try to fix the test_getActions later.

[osimons]

Superb, Jun. Looks good to me!

Olemis, I seem to remember you having reported that particular test_getActions failure before in some other context, but I have yet to replicate it. At my end tests are passing for all ~recent Trac branches (0.11++). Hard to research for me without actual test failure...

Jun, I'm ready to integrate your changes. Or should I perhaps set you up with write permission to the plugin so that you can push the changes yourself? Your involvement is always appreciated! ;-)

[Jun Omae]

(In [13193]) XmlRpcPlugin: Fixed 'maximum recursion depth exceeded' error on search.performSearch with JSON-RPC. Closes #10786.

[Jun Omae]

Thanks, Simon and Olemis! I've directly pushed in [13193].

[osimons]

Goodie. But the tracrpc/tests/search.py file seems not to have quite made it across from Git til Subversion. No file added in [13193].

[Jun Omae]

Ouch. Added in [13194], sorry.

[Olemis Lang]

Replying to jun66j5:

Replying to olemis:

|| trunk || http://pastebin.com/yEjMttXH ||

The failure in test_getActions brings the other two failures. Also, the failure happens with xmlrpcplugin/trunk-r12546, too. Then, that is another issue.

yes , now I noticed ... :-/

Anyway, I made test_fragment_in_search more robust. See ​https://github.com/jun66j5/xmlrpcplugin/compare/trunk...ticket10786 (​diff).

I'll try to fix the test_getActions later.

@jun66j5 : So this means you are also experiencing the same issue regarding test_getActions ?

If so , what's the version of your python interpreter ? /me testing with python=2.6

[Jun Omae]

Replying to olemis:

@jun66j5 : So this means you are also experiencing the same issue regarding test_getActions ?

Yes. I get the same failures as ​http://pastebin.com/wKqeJivW with 1.0-stable.

If so , what's the version of your python interpreter ? /me testing with python=2.6

Python 2.5 and 2.6. For Python 2.4, tracrpc/tests/api.py has an syntax error. Also, All tests pass on Trac 0.12 - 0.12.5, 0.12-stable and 1.0 - 1.0.1 with Python 2.5 and 2.6.

[Olemis Lang]

Replying to jun66j5:

Replying to olemis:

@jun66j5 : So this means you are also experiencing the same issue regarding test_getActions ?

Yes. I get the same failures as ​http://pastebin.com/wKqeJivW with 1.0-stable.

ok , please see #11109 for further details ;)

[...]

For Python 2.4, tracrpc/tests/api.py has an syntax error.

I'm of the opinion that support for python=2.4 should be removed in recent versions of the plugin ... objections ?

[...]

[osimons]

It is my intention to support all versions of Python supported by the corresponding versions of Trac. As long as the plugin (trunk) still supports 0.11 and 0.12, Python2.4 is a requirement as I see it. In theory even 2.3 is supported, but it's been a long time since I've had that installed ;-)

I intend to branch for future major incompatible changes, but until actually required I'm happy to add the necessary compat code to make this work across versions. I very much prefer one codebase, and don't subscribe to the opinion that plugins should follow Trac branching. Breaking changes to the RPC API or implementations are more valid reasons to branch, IMO. And if we start branching for each combination of major Trac version + internal RPC changes, everything would quickly become a confusing mess...

However, as correctly spotted, the plugin did indeed have Python 2.4 issues - but only in the tests, not in the actual RPC code/handlers. I've fixed it in [13201].

[Olemis Lang]

Replying to osimons:

It is my intention to support all versions of Python supported by the corresponding versions of Trac. As long as the plugin (trunk) still supports 0.11 and 0.12, Python2.4 is a requirement as I see it.

Considering this , I've just changed my mind on this subject . You are right .

I intend to branch for future major incompatible changes, but until actually required I'm happy to add the necessary compat code to make this work across versions. I very much prefer one codebase, and don't subscribe to the opinion that plugins should follow Trac branching. Breaking changes to the RPC API or implementations are more valid reasons to branch, IMO. And if we start branching for each combination of major Trac version + internal RPC changes, everything would quickly become a confusing mess...

of course , I get your point , and agree with you .

However, as correctly spotted, the plugin did indeed have Python 2.4 issues - but only in the tests, not in the actual RPC code/handlers. I've fixed it in [13201].

:)