[dev] Horde_RPC_soap tweaks

Wed Nov 28 19:00:11 UTC 2007

Sorry for the huge time gap; free time and all that. :-/ I saw Jan's recent
call for RC1 bugs/features and wanted to see if there's time to get these
changes in for the 3.2 release.

I tried piecing together the relevant parts of the thread (below) and added
my comments; hopefully it's not too disorganized to follow. The original
thread is at:

http://marc.info/?l=horde-dev&m=117951223322085&w=2

if you'd rather go through the handful of messages in their original form.

On Tue, May 22, 2007 at 12:30:41AM +0200, Jan Schneider wrote:
> On Mon, May 21, 2007 at 01:33:24PM -0400, Chuck Hagenbuch wrote:
> > Quoting John Morrissey <jwm at horde.net>:
> > > 1. Adds a restrictCalls parameter, a regex that calls must match to
> > >    be allowed.
> >
> > This one I have a gut reaction against, though I certainly see your
> > use case. I'm not sure how widespread that need is, though, and I feel
> > like having more fine-grained control over which methods are exposed,
> > for all backends, would be better. Unlikely that can be done without
> > breaking BC though, so I'm neutral on this for now.
> 
> We actually discussed this earlier under different circumstances, to  
> add an additional parameter to the api descriptions that turns certain  
> methods off for the external interfaces. This could even be done BC.
> It was supposed to be a boolean setting, but you could as well turn it  
> into a setting that accepts endpoint names.

I like this idea better (access controls in the api.php descriptions), but
how could we identify the current endpoint? The URL is the first thing that
comes to mind, but matching that seems to be error prone (hostname vs. IP
address, rewritten paths to rpc.php?wsdl, use of Horde_SOAP_rpc with
non-HTTP transports, etc.) You could always require that all possible URLs
are listed, but that gets really verbose and seems to leave the admin open
for surprises when IP addresses are added, etc. It also doesn't handle the
non-HTTP case.

On Tue, May 22, 2007 at 12:30:41AM +0200, Jan Schneider wrote:
> Did you try setting the available methods explicitly in the 'provides'  
> settings in registry.php instead?

Yup. However, Horde_RPC_soap calls Registry::listMethods() with no
arguments, so all methods are listed. It could be modified to pass (for
example) the name of the current app, but that would limit listMethods()
output to only calls for that app.

On Mon, May 21, 2007 at 11:50:29PM -0400, Chuck Hagenbuch wrote:
> On Tue, May 22, 2007 at 12:30:41AM +0200, Jan Schneider wrote:
> > Zitat von John Morrissey <jwm at horde.net>:
> > > So you're thinking of a programmatic or configurable way to specify
> > > which API calls are exposed via any/all RPC methods? That would be
> > > useful, but to handle the use case I mentioned, we'd still need a way
> > > to selectively expose certain calls *only* via certain endpoints (not
> > > backends, endpoints).
> > >
> > > To give you an idea, to create our customer-facing SOAP endpoint, we
> > > copied horde's rpc.php and modified it for SOAP-only use, using a
> > > restrictCalls argument in only that copy of rpc.php. The "internal
> > > use" endpoint uses horde's unmodified rpc.php and the customer-facing
> > > endpoint uses the modified rpc.php.
> > 
> > I already wondered where you are going to provide the endpoint  
> > parameter. While being able to set the endpoint is nice, you still  
> > need to hack the code to use it. I'm against introducing a parameter  
> > that actually can't be used inside Horde.
> 
> This is a feature for pretty advanced users; no reason they can't have  
> their own rpc.php endpoints. Also, it's an addition, not necessary to  
> really hack the existing install.

Any further thoughts on this aspect?

On Mon, May 21, 2007 at 04:55:55PM -0400, Chuck Hagenbuch wrote:
> Quoting John Morrissey <jwm at horde.net>:
> > Chuck Hagenbuch wrote:
> > > This one I have a gut reaction against, though I certainly see your
> > > use case. I'm not sure how widespread that need is, though, and I feel
> > > like having more fine-grained control over which methods are exposed,
> > > for all backends, would be better. Unlikely that can be done without
> > > breaking BC though, so I'm neutral on this for now.
> >
> > So you're thinking of a programmatic or configurable way to specify
> > which API calls are exposed via any/all RPC methods? That would be
> > useful, but to handle the use case I mentioned, we'd still need a way to
> > selectively expose certain calls *only* via certain endpoints (not
> > backends, endpoints).
> 
> Yeah. I guess after thinking about this, I'd rather you passed an  
> explicit array of allowed methods in the RPC constructor params  
> instead of using a regex. The regex just feels hackish to me, but  
> creating a limited server doesn't. How does that sound?

That sounds decent, unless we decide on a straightforward way of specifying
API method visibility in api.php (above).

> > I was torn between _INFO and _DEBUG. On one hand, these logs are a lot
> > like web server access logs in that they're useful for after-the-fact
> > troubleshooting or usage-pattern profiling ("oh, so the box is 300MB
> > into swap because $REMOTE_APPLICATION started throwing 10x the normal
> > SOAP transaction load at us"). This use doesn't seem amenable to _DEBUG
> > since _DEBUG turns up the verbosity significantly, such as with
> > individual SQL queries, which don't seem that useful on an ongoing
> > basis.
> >
> > Oh, and I forgot to mention that this logging doesn't do a deep array
> > walk to log arguments. If any arguments are arrays, the logging will
> > cast them to the string Array. I didn't think this was a huge deal since
> > we can never be sure how deep an array goes.
> 
> Alright, convinced - just as long as the INFO version is really very  
> concise. Could have a DEBUG version with more info if necessary.

On Tue, May 22, 2007 at 12:30:41AM +0200, Jan Schneider wrote:
> This is definitely a DEBUG log.

It's fairly concise; here's sample output:

SOAP call: appname/searchAccounts('*', Array) by user at example.com serviced
in 1 seconds, sent 609 bytes in response

This has been very useful when troubleshooting load problems or when proving
to other groups that our application is behaving properly. I liken it to
Apache's access logs; they're highly useful to determine the current and
past operating state of the machine. Like I mentioned, LOG_DEBUG brings in a
lot of other verbose logging, like SQL queries, that are substantially less
useful than the SOAP logging has been for us.

> And is there any reason why you use global vars instead of object
> properties?

I tried object properties initially, but it seems that something was
operating on a copy of the Horde_RPC_soap instance, so the data set in
Horde_RPC_soap::_dispatcher() never made it to Horde_RPC_soap::getResponse()
for logging. I suspect something in the PEAR SOAP code, but wasn't able to
find where.

john
-- 
John Morrissey          _o            /\         ----  __o
jwm at horde.net        _-< \_          /  \       ----  <  \,
www.horde.net/    __(_)/_(_)________/    \_______(_) /_(_)__