Jim Gallacher wrote:
> Max Bowsher wrote:
>> Jim Gallacher wrote:
>>> The mod_python 3.2.9-rc2 tarball is available for testing.
>> Something about the mod_python.util changes has either exposed a bug in
>> Trac, or introduced a bug into mod_python - I'm not sure which yet.
>>
>> 3.2.x r416547 with r393781 reverted works fine for me
>> 3.2.x r416548 seems to be behaving as if any path info in the URL beyond
>> the object designated as a mod_python handler is being truncated after
>> the first slash - i.e.:
>>
>> If the configuration is:
>>
>> <Location /trac>
>>   SetHandler mod_python
>>   ...
>> </Location>
>>
>> then an URL like:
>>
>> http://host/trac/foo/bar/baz
>>
>> gets treated as:
>>
>> http://host/trac/foo/
>>
>>
>> I'll try to narrow down the source of the problem.
> 
> I installed trac and can reproduce the behaviour. The latest changes to
> Field *should* work, so I'm thinking something is a little wonky in
> FieldStorage. No time right now but I'll be able to dig into it later
> today - if you haven't found the solution by then Max. ;)

OK, I've found the solution.

The root of the problem is that Trac wants to be able to add extra
fields to a FieldStorage itself, and has been jumping through all sorts
of crazy hoops in the internals of FieldStorage to make this happen.
The recent changes have moved all the hoops, and Trac is now failing
utterly.

First problem, is that the new FieldStorage memoizes self.dictionary the
first time it creates it. However, Trac expects to be able to append to
FieldStorage.list to add new fields. Since the list member is documented
in the public API docs, this isn't entirely unreasonable. In any case,
list ought to have a leading underscore if it is supposed to be private.

OK, so suppose you comment out the line "self.dictionary = result", so
that the dictionary gets rebuilt every time it is needed. Does it work? No!

The fun doesn't stop there. The second problem is related to the way
that pre-3.2.9 mod_python likes to stuff everything into unnecessary
StringIO objects. Trac tries to replicate this, and this causes problems
since 3.2.9 no longer expects StringIO objects everywhere.  Trac is
actually relying on pre-3.2.9 behaviour of transforming
string-in-StringIO Fields into StringFields during
FieldStorage.__getitem__, but this no longer happens in 3.2.9.


Here is the patch that seems to make things work again:
--- util-bad.py 2006-06-23 08:12:00.000000000 -0500
+++ util.py     2006-06-23 09:51:32.000000000 -0500
@@ -323,10 +323,17 @@
    def __getitem__(self, key):
        """Dictionary style indexing."""
        found = self.dictionary[key]
-       if len(found) == 1:
-           return found[0]
+       newfound = []
+       for item in found:
+         if isinstance(item.file, FileType) or \
+             isinstance(getattr(item.file, 'file', None), FileType):
+             newfound.append(item)
+         else:
+             newfound.append(StringField(item.value))
+       if len(newfound) == 1:
+           return newfound[0]
        else:
-           return found
+           return newfound

    def get(self, key, default):
        try:
@@ -374,7 +381,6 @@
        if list is None:
            raise TypeError, "not indexable"
        result = {}
-       self.dictionary = result
        for item in list:
            if item.name in result:
               result[item.name].append(item)



Yes, ugh.

So, I guess the two questions to answer now are:

* How much non-compatibility is acceptable in a patch release?

* How are applications supposed to perform write operations on a
FieldStorage, in 3.3 and the future?

Max.

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to