Opened 5 years ago

Closed 4 years ago

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

Reported by: Owned by: Marco Antonio Islas Cruz Marco Antonio Islas Cruz normal XmlRpcPlugin blocker search Olemis Lang, Jun Omae 1.0

### Description

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.

### comment:1 follow-up:  2 Changed 5 years ago by 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.

### comment:2 in reply to:  1 ; follow-up:  3 Changed 5 years ago by Olemis Lang

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

### comment:3 in reply to:  2 Changed 5 years ago by anonymous

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

### comment:4 follow-up:  6 Changed 5 years ago by Odd Simon Simonsen

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:

### comment:5 Changed 5 years ago by Marco Antonio Islas Cruz

Owner: changed from Odd Simon Simonsen to Marco Antonio Islas Cruz new → assigned

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

### comment:6 in reply to:  4 ; follow-up:  7 Changed 4 years ago by Jun Omae

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.

### comment:7 in reply to:  6 ; follow-up:  8 Changed 4 years ago by Olemis Lang

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

### comment:8 in reply to:  7 ; follow-up:  9 Changed 4 years ago by Jun Omae

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?

### comment:9 in reply to:  8 ; follow-up:  10 Changed 4 years ago by Olemis Lang

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

### comment:10 in reply to:  9 ; follow-up:  16 Changed 4 years ago by Jun Omae

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

### comment:11 Changed 4 years ago by Odd Simon Simonsen

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! ;-)

### comment:12 Changed 4 years ago by Jun Omae

Resolution: → fixed assigned → closed

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

### comment:13 Changed 4 years ago by Jun Omae

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

### comment:14 Changed 4 years ago by Odd Simon Simonsen

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

### comment:16 in reply to:  10 ; follow-up:  17 Changed 4 years ago by Olemis Lang

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

### comment:17 in reply to:  16 ; follow-up:  18 Changed 4 years ago by Jun Omae

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

### comment:18 in reply to:  17 Changed 4 years ago by Olemis Lang

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

[...]

### comment:19 follow-up:  20 Changed 4 years ago by Odd Simon Simonsen

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

### comment:20 in reply to:  19 Changed 4 years ago by Olemis Lang

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

:)

### Modify Ticket

Change Properties