search

crs-tools / tracker

CRS Ticket Tracker
Apache License 2.0
18 stars 11 forks source link

XMLRPC: errors not caught, creating invalid XML response #209

Closed pegro closed 5 years ago

pegro commented 5 years ago

When calling API functions with invalid arguments, PHP errors are thrown but not handled by the XMLRPC controller.

Argument 2 passed to Controller_XMLRPC_Handler::resetTicket() must be of the type array, string given

An easy fix would be to catch Throwables instead of Exceptions in XMLRPC.php:89, but then information about internal workings of the code are exposed, when returning the PHP error message in the response.

So maybe add a second catch statement and return a generic "Bad request" error? Opinions?

a-tze commented 5 years ago

Opinion: return the actual error message (as in the example above) as error message via XMLRPC, but put backtrace etc. in the logfile.

jjeising commented 5 years ago

as error message via XMLRPC, but put backtrace etc. in the logfile.

That would already be the case, if we implement the suggested fix by @pegro. I think this would be reasonable as our code is already public and there should not be any other errors. Maybe we should catch DatabaseException and remove the message in this case (but it was already exposed in the past).

pegro commented 5 years ago

Most errors regarding invalid arguments thrown in the controller or model are fine to be returned to the user I think. DatabaseExceptions are rarely useful to the API user, without reading the source code. So I guess @jjeising suggestion is reasonable.

jjeising commented 5 years ago

@pegro I've tried to implement the suggested fix, could you test if this helps?