An initial few comments from a first pass through.

    def _write(self, request, response, content_type='text/xml'):
        request.send_http_header()
        request.content_type = content_type
        request.write(response)

This is technically wrong, although it doesn't matter on mod_python > 3.0.

The issue is that send_http_header() in mod_python 2.7 should only be
called after content type is set. Here they do it before. Only works because
in 3.X send_http_header() is a NOP.

        # men have been killed for less
        temp = sys.stderr
        sys.stderr = stderr_mod_python(self.request)

        ...

        sys.stderr = temp

This is not safe in a multithread MPM.

        except Exception, e:
            # report exception back to server
            response = xmlrpclib.dumps(
                xmlrpclib.Fault(1, "%s:%s" % (sys.exc_type, sys.exc_value))
                )
            # and also log it, duh
            etype, evalue, etb = sys.exc_info()
            
            stack = traceback.format_exception(etype, evalue, etb)             
            for l in stack:
                sys.stderr.write(l)

First it uses fudged sys.stderr. Second is that it exacerbates a problem in
XML-RPC which is that there is no concept of namespaces for error return
codes. Because they have used arbitrary return status of "1" for internal
exception or unexpected exception in user code, then you can't distinguish
easily a valid fault response with return status of "1" generated by user
code from unexpected exception.

It may be more appropriate to generate a 500 HTTP error response in
this circumstance given that it really consitutes an internal server error
rather than it being a valid XML-RPC fault response generated by the
user. This is an issue for debate though. It depends on whether you want
to be conformant with how SimpleXmlRpcServer works which is where
this questionable code came from in the first place.

Other issues are that it doesn't check incoming content type to validate
that it is actually 'text/xml' per specification for XML-RPC. It doesn't use
incoming content length for read on POST data which can be a problem
in some cases. It also doesn't set outgoing content length as per
specification for XML-RPC. It could also perhaps be a bit more knowledgeable
about mod_python and pass through apache.SERVER_RETURN exception
so as to allow exposed methods to still generate it if need be.

This isn't the only implementation of XML-RPC support integrated with
mod_python. I have an alternate take on it in Vampire which isn't bound
to the SimpleXmlRpcServer base class. See:

  http://svn.dscpl.com.au/vampire/trunk/software/vampire/xmlrpc.py

I can't find the others right now, but I have posted links to them before
on the main mod_python list.

Overall, I'm not sure at this point that it is worthwhile putting XML-
RPC support in mod_python. If it is done, I would prefer to see it be
done as part of a larger effort to provide a range of handler components
which all work consistently together, rather than adhoc bits and pieces
that cannot be glued together easily.

Grisha wrote ..
> 
> If someone here has spare Brain/CPU cycles, could you look at the attached
> code and provide feedback?
> 
> Grisha
> 
> ---------- Forwarded message ----------
> Date: Mon, 30 Jan 2006 18:04:42 -0800
> From: Matt Chisholm <[EMAIL PROTECTED]>
> Subject: Re: contribution to mod_python: Apache + SimpleXMLRPCServer
> 
> On Jan 30 2006, 11:42, Matt Chisholm wrote:
> >We've written a few classes to use the SimpleXMLRPCServer module in
> >Python with mod_python instead of the Python CGI module.  We've been
> >using it internally for a while and we'd like to contribute it back to
> >the mod_python project; we don't really have the time to create a
> >separate project for it, but it seems like something that would be
> >useful to many people.
> >
> >We agree to assign copyright of this code to the mod_python project
> >and to license it under the mod_python license.  Our employer,
> >BitTorrent Inc., also agrees.
> >
> >I've attached a copy of the code.
> >
> >Please let me know if this is not the right channel to send
> >contributions; also, I'm not on this list so please respond to me
> >individually.
> >
> >Matt Chisholm

Reply via email to