Rereading Trac code, the req.args in constructor is still an issue as what the code is actually doing is prevent util.FieldStorage processing req.args and for it to do it itself, only adding args where they aren't also defined in
POST body.

The only way to provide a fiddle for this would be to use a special derived instance of the list object which redefines append and translates arguments
as necessary on the fly so it would be compatible.

Graham

On 20/10/2006, at 10:08 AM, Graham Dumpleton wrote:

Jim Gallacher wrote ..
         # Populate FieldStorage with the original query string
parameters, if
         # they aren't already defined through the request body
         if req.args:
             qsargs = []
             for pair in util.parse_qsl(req.args, 1):
                 if self.has_key(pair[0]):
                     continue
                 qsargs.append(util.Field(pair[0], StringIO(pair[1]),
"text/plain", {}, None, {}))
             self.list += qsargs

     def get(self, key, default=None):
         # Work around a quirk with the ModPython FieldStorage class.
# Instances of a string subclass are returned instead of real
         # strings, this confuses psycopg2 among others.
         v = util.FieldStorage.get(self, key, default)
         if isinstance(v, util.StringField):
             return v.value
         else:
             return v

     def __setitem__(self, key, value):
         if value is not None and key not in self:
self.list.append(util.Field(key, StringIO(value), 'text/plain',
                              {}, None, {}))

Presuming I am correct, I interpret the problem being with __setitem__().
This is because the req.args in constructor shouldn't matter as latest
mod_python FieldStorage adds them anyway so it shouldn't try and add
them again.

If I remember, someone said that Trac added extra key/values after the
FieldStorage instance is created, presumably by __setitem__ being invoked.
Thus, how the badly constructed data gets in there.

Is this correct?

I know it is a real hack, but we could perhaps change the constructor of
util.FieldStorage to include:

  self.__setitem__ = self.add_field

Since the __setitem__ in the derived class has already been added to the instance at this point, this line in the util.FieldStorage class effectively
overrides it and we can replace it with the method that actually does
what is required.

For a working test, try:

class X:
  def __init__(self):
    self._dict = {}
    #self.__setitem__ = self.add_field
  def add_field(self, key, value):
    self._dict[key] = value
  def __getitem__(self, key):
    return self._dict[key]

class Y(X):
  def __init__(self):
    X.__init__(self)
  def __setitem__(self, key, value):
    self._dict[key] = (value, value)

y = Y()

y._dict[1] = 1
print y[1]

y[2] = 2
print y[2]

Run that and then uncomment the override and see how now uses the
base class add_field.

Okay, its a hack, but in practice would it be a workable solution?

Adding the ability to use [] instead of add_field() may be a desirable
feature for the default implementation anyway. Doing it this way we
fix Trac in the process presuming I am correct about req.args in the
constructor not being an issue.

Graham

Reply via email to