dmwm / PHEDEX

CMS data-placement suite
8 stars 18 forks source link

Dataservice APIs sometimes return, sometimes die, for invalid arguments #854

Open ericvaandering opened 10 years ago

ericvaandering commented 10 years ago

Original Savannah ticket 91144 reported by wildish on Fri Feb 3 05:23:26 2012.

Some data-service APIs die for parameter-validation failures, some simply return a formatted error object. We need a consistent policy here, then we need to adapt the clients to follow it.

On the one hand, it's more semantically correct that an argument validation simply return, not die. Death should be reserved for exceptions, and invalid arguments are not really that.

On the other hand, if they simply return a formatted PHEDEX::Web::Util::http_error string, the client is then left with the problem of figuring out what to do about it. It has to examine the returned object to see if it's a string or a hashref, and if it's a string, it has to decide how to handle it.

This will affect both the existing website, where some modules are called directly, and the nextgen website, where error messages from the data-service are expected to be meaningful - and not to be an HTML document!

A related topic is that the error messages from validation failures contain the value of the parameter as well as its name. This can lead to html-injection problems if the error message is used improperly. We still need to log the failed values for auditing, but should not simply tell the user that the parameter was not valid.

So there are actually three issues here: 1) decide if we die or return from APIs? (I prefer return) 2) in the data-service, return a formatted error document or plain text? (I prefer plain-text) 3) log the failed parameter-value in the logfiles, but remove it from the string returned to the client

ericvaandering commented 10 years ago

Comment by wildish on Thu Feb 9 09:32:35 2012

one direct corollary of this is that the UpdateRequest API, which is called explicitly (and internally) by the old website, can die with meaningful error messages (e.g. 'you are not authorised ...'), but these will be masked by the site and returned to the user as a generic error. We need to decide how to handle this.