Hang on subscripted access to request.subprocess_env.

• Key: MODPYTHON-187
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Graham Dumpleton
• Reporter: Alan Kennedy
• Component/s: core
• Affects Version/s: 3.2.10
• Fix Version/s: 3.3.1

Environment

Apache: 2.0.59
ModPython: 3.2.10
Python: 2.4
OS: Windows Server 2003

Description

When subscripted access is used to access variable 'SCRIPT_FILENAME' in the request.subprocess_env table/mapping, the code hangs.

The following snippet illustrates the problem.

# -=-=-=-=-=-=

def postreadrequesthandler(request):
  request.add_common_vars()
  value = request.subprocess_env['SCRIPT_FILENAME'] # This hangs
# value = request.subprocess_env.get('SCRIPT_FILENAME')# This works
  return apache.OK

# -=-=-=-=-=-=

The strange thing is that the .get() access works fine: only the subscript hangs?

If anyone is wondering about a use-case, I don't actually have one. I was just iterating over the contents of the request.subprocess_env using a for loop (code below), and found that the code hung when accessing 'SCRIPT_FILENAME', and not for any other variable.

# -=-=-=
request.add_common_vars()
d = {}
for sek in request.subprocess_env.keys():
  d[sek] = request.subprocess_env[sek]
# -=-=-=

Comments

[Jim Gallacher]

Confirmed for Apache 2.0.55 mpm-worker (Linux Ubuntu 6.0.6), execpt I get a segfault rather than a hang.

Note that the Post Read Request phase occurs before apache translates the URI to a filename. request.subprocess_env.get('SCRIPT_FILENAME') seems to do the right thing by returning None.

The problem seems to be in the tableobject.c function table_subscript().

subprocess_env.get['SCRIPT_FILENAME'] is being evaluated in the post read request phase. It would seem that req.add_common_vars adds a key for SCRIPT_FILENAME to the table, but since the filename has not been translated yet its value is NULL.

table_subscript() retrieves the value with the following bit of code:

   while (i--)
        if (elts[i].key) {
            if (apr_strnatcasecmp(elts[i].key, k) == 0) {
                PyObject *v = PyString_FromString(elts[i].val);
                PyList_Insert(list, 0, v);
                Py_DECREF(v);
            }
        }

The problem is with the PyString_FromString. According to the python docs, the parameter passed must not be NULL and it will not be checked. I assume this is the origin of the segfault.

Something like the following seems to fix the problem.

The following code snippet seems to fix the problem, and is attached as MP187-2006-08-28-jgallacher-1.diff. Am I correct in doing the PY_INCREF(v)??

     while (i--)
         if (elts[i].key) {
             if (apr_strnatcasecmp(elts[i].key, k) == 0) {
- PyObject *v = PyString_FromString(elts[i].val);
+ PyObject *v = NULL;
+ if (elts[i].val != NULL) {
+ v = PyString_FromString(elts[i].val);
+ } else {
+ v = Py_None;
+ Py_INCREF(v);
+ }
                 PyList_Insert(list, 0, v);
                 Py_DECREF(v);

[Jim Gallacher]

I hope I have the reference count right!

[Jim Gallacher]

I've audited tableobject.c for other lines where NULL is being passed to PyString_FromString and found a few functions that may segfault if they are called in the post read request phase where SCRIPT_FILENAME is NULL.

Line numbers are for version 3.3.0-dev-20060827, r437300.

line 178 in table_repr()
            PyString_ConcatAndDel(&s, PyString_FromString(elts[i].val));


line 366 in function table_values()
            PyObject *val = PyString_FromString(elts[i].val);

line 792 in function table_traverse()
            PyObject *v = PyString_FromString(elts[i].val);

line 845 in function select_value()
    return PyString_FromString(elts->val);

We should audit all our code to make sure we are not making the same mistake with PyString_FromString elsewhere.

[Graham Dumpleton]

Although table_traverse() was changed, not sure it is every called as garbage collecting not enabled for this class type.