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