PythonHandlerModule directive is broken.

• Key: MODPYTHON-46
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Unassigned
• Reporter: Graham Dumpleton
• Component/s: core
• Affects Version/s: 3.1.4
• Fix Version/s: 3.2.7

Description

Documentation for PythonHandlerModule says:

  PythonHandlerModule can be used an alternative to Python*Handler directives.
  The module specified in this handler will be searched for existence of functions
  matching the default handler function names, and if a function is found, it will
  be executed.

The suggestion is that it will not complain if a particular handler is defined, ie.,
only executes the ones it finds and doesn't worry about the rest. The example
even supports this by saying that:

  For example, instead of:

    PythonAutenHandler mymodule
    PythonHandler mymodule
    PythonLogHandler mymodule

  one can simply say

    PythonHandlerModule mymodule

BTW, "PythonAutenHandler" is spelt wrong in documentation, not by me.

The "mod_python.c" code also seems be coded so that if a handler is defined
in the module that it will not complain.

    python_directive_handler(cmd, mconfig, "PythonPostReadRequestHandler", val, SILENT);
    python_directive_handler(cmd, mconfig, "PythonTransHandler", val, SILENT);
    python_directive_handler(cmd, mconfig, "PythonHeaderParserHandler", val, SILENT);
    python_directive_handler(cmd, mconfig, "PythonAccessHandler", val, SILENT);
    python_directive_handler(cmd, mconfig, "PythonAuthzHandler", val, SILENT);
    python_directive_handler(cmd, mconfig, "PythonTypeHandler", val, SILENT);
    python_directive_handler(cmd, mconfig, "PythonHandler", val, SILENT);
    python_directive_handler(cmd, mconfig, "PythonInitHandler", val, SILENT);
    python_directive_handler(cmd, mconfig, "PythonLogHandler", val, SILENT);
    python_directive_handler(cmd, mconfig, "PythonCleanupHandler", val, SILENT);
    python_directive_handler(cmd, srv_conf, "PythonConnectionHandler", val, SILENT);

Ie., it has "SILENT" option and not "NOTSILENT" as is case when single handler is
specified.

Problem is that using "PythonHandlerModule" it gives back 500 error and if
"PythonDebug" is on you will see in the browser:

  Mod_python error: "PythonHeaderParserHandler mptest"

  Traceback (most recent call last):

    File "/usr/lib/python2.3/site-packages/mod_python/apache.py", line 291, in HandlerDispatch
      arg=req, silent=hlist.silent)

    File "/usr/lib/python2.3/site-packages/mod_python/apache.py", line 519, in resolve_object
      raise AttributeError, s

  AttributeError: module '/home/grahamd/public_html/phases/mptest.py' contains no 'headerparserhandler'

The passing of "SILENT" thus seems to not work.

The definitions of SILENT and NOTSILENT are:

  #define SILENT 0
  #define NOTSILENT 1

This eventually gets set as hlist.silent and gets passed as "silent" argument of
the "resolve_object()" method.

In the resolve_object() call of apache.py where this is checked, it is checked as:

        # don't throw attribute errors when silent
        if silent and not hasattr(obj, obj_str):
            return None

        # this adds a little clarity if we have an attriute error
        if obj == module and not hasattr(module, obj_str):
            if hasattr(module, "__file__"):
                s = "module '%s' contains no '%s'" % (module.__file__, obj_str)
                raise AttributeError, s

Is the logic the wrong way around here or am I just going nuts?

The result of "resolve_object()" is used as:

                if object:
                    ...

                elif hlist.silent:
                    result = DECLINED

This is supposed to propogate ignoring of the fact that the handler is missing,
but again logic is wrong way.

The simple solution may be:

  #define NOTSILENT 0
  #define SILENT 1

All uses of this silent flag needs to be reviewed though to determine if this is
going to stuff up other areas of the code.

Comments

[Graham Dumpleton]

Conversely, if you define:

  PythonHandler xxx::yyy

and "yyy" doesn't exist in the module "xxx", it doesn't complain when
it really should.

[Graham Dumpleton]

Even when one works around this. There seems to be a bug deep
in the C code somewhere, whereby on occasion a request gets
stuck in an infinite loop when processing the PythonHandlerModule
directive. The stack trace is:

  directive_PythonHandlerModule
  python_directive_handler
  python_directive_hl_add
  hlist_append

Ie., gets stuck in a loop in hlist_append.

This comment is here just so I don't forget about it as not going to try and
track it down right now.

This fixes the wrong logic problem in the definition of SILENT/NOTSILENT.

It avoids the infinite loop bug described by not having PythonHandlerModule do anything for "PythonConnectionHandler". The "PythonConnectionHandler" is different because the handler is being added against the handler list of the server config object and not module config. The bug is somehow associated with the server config object although still don't know why.

Since PythonHandlerModule is broken altogether at the moment, I don't see a problem with it being made to work for normal module handlers and ignore the connection handler, at least for now. It could even be possible that PythonConnectionHandler directive is broken as well but since no one uses it, no one has noticed.

The most important thing here is that the reverse logic in SILENT/NOTSILENT will be fixed and saying:

  PythonHandler mptest::xxx

where "xxx" doesn't exist, will yield an error, whereas at present no error results and instead "mptest.py" source code could be served up instead.

[Jim Gallacher]

I've reviewed the source code and according to src/include/hlist.c:

    typedef struct hl_entry {
        const char *handler;
        const char *directory;
        int silent; /* 1 for PythonHandlerModule, where
                        if a handler is not found in a module,
                        no error should be reported */
        struct hl_entry *next;
    } hl_entry;

So Graham is correct. The logic of SILENT/NOTSILENT is reversed.

Also it looks like SILENT/NOTSILENT is only used in the context of python_directive_handler, so I don't see any side effects from making this change.

I have no objection to disabling PythonConnectionHandler within PythonHandlerModule. If there are no objections I'll apply Graham's patch.