Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #1407 (closed defect: fixed)

Opened 10 years ago

Last modified 10 years ago

InternalRedirect conflict with nested variables and decoding filter

Reported by: simonk Owned by: anonymous
Priority: normal Milestone: 1.0.4
Component: TurboGears Version: trunk
Severity: normal Keywords:
Cc: simon.king@…

Description

(I think this is related to ticket #1340, but I don't want to hijack someone else's ticket without permission - please feel free to close as a duplicate if you think it is appropriate)

I get a problem when I try to access a URL protected by Identity with parameters that are parsed by the nested variables filter (eg. /protected_url?a.b=1&a.c=2), when I am not logged in. Instead of being directed to the login page, I get a stack trace from the decoding filter.

The attached patch to the identity test suite should hopefully show the problem. Basically, I think the CherryPy? decoding filter is running after the nested variables filter when an Identity check is involved, and it doesn't know how to deal with dicts. The stack trace is:

Traceback (most recent call last):
  File "C:\Python24\lib\site-packages\cherrypy-2.2.1-py2.4.egg\cherrypy\_cphttptools.py", line 103, in _run
    applyFilters('before_main')
  File "C:\Python24\lib\site-packages\cherrypy-2.2.1-py2.4.egg\cherrypy\filters\__init__.py", line 151, in applyFilters
    method()
  File "C:\Python24\lib\site-packages\cherrypy-2.2.1-py2.4.egg\cherrypy\filters\decodingfilter.py", line 31, in before_main
    self.decode(enc)
  File "C:\Python24\lib\site-packages\cherrypy-2.2.1-py2.4.egg\cherrypy\filters\decodingfilter.py", line 50, in decode
    decodedParams[key] = value.decode(enc)
AttributeError: 'dict' object has no attribute 'decode'

Since ticket #1340 is about unicode strings rather than nested variables, is it possible that the filters are actually being run twice when redirecting to the failure_url, causing the decode filter to decode things that are already unicode strings (the problem in #1340) or dictionaries (in the nested variables case).

Attachments

identity.patch Download (714 bytes) - added by simonk 10 years ago.
Patch to identity test suite to demonstrate the problem
internal_redirect.patch Download (1.4 KB) - added by simonk 10 years ago.
Patch to test suite demonstrating problem with InternalRedirect?

Change History

Changed 10 years ago by simonk

Patch to identity test suite to demonstrate the problem

Changed 10 years ago by simonk

Patch to test suite demonstrating problem with InternalRedirect?

comment:1 Changed 10 years ago by simonk

  • Summary changed from Identity conflict with nested variables and decoding filter to InternalRedirect conflict with nested variables and decoding filter

I've looked a little further into this, and it looks like the problem is caused by the use of InternalRedirect? (the IdentityFailure? exception is a subclass of cherrypy.InternalRedirect?). It is possible to trigger the problem without Identity by raising InternalRedirect? in a controller method which has had a parameter converted to a dict.

It looks like CherryPy? explicitly calls filters again after an internal redirect. In _cphttptools:

          # Loop to allow for InternalRedirect.
          while True:
              try:
                  applyFilters('before_main')
                  if self.execute_main:
                      self.main()
                  break
              except cherrypy.InternalRedirect, ir:
                  self.object_path = ir.path

I haven't checked, but I would have thought this would cause a problem with values that have been converted by validators as well, wouldn't it? For example, if you've converted a string to an integer, won't that cause a problem?

comment:2 Changed 10 years ago by simonk

OK, further searching around has turned up ticket #1022, which references  http://www.cherrypy.org/ticket/590. However, #1022 has been marked as closed/invalid, which I'm pretty sure is wrong. I think this is something that needs to be fixed in TurboGears. One way to fix it would be to ensure that after an InternalRedirect?, parameters are completely reparsed from the original request, instead of using values that have already been converted. This could cause a problem if people were causing side-effects during parameter parsing, but I think that would be pretty bad style anyway.

cherrypy.request.original_params is useful here. It can be passed as the 'params' argument to InternalRedirect? but it seems to be deprecated - I don't know whether it has an equivalent in CP3. Also, it triggers the #1340 problem, because it is already a unicode string, so it shouldn't be decoded...

comment:3 Changed 10 years ago by faide

  • Milestone changed from 1.0.3 to 1.1

comment:4 Changed 10 years ago by faide

  • Status changed from new to closed
  • Resolution set to fixed

Fixed in #3771. Thanks for the provided tests... Tests are always _greatly_ appreciated!

comment:5 Changed 10 years ago by roger.demetrescu

Correction: it is fixed in [3771]

comment:6 Changed 10 years ago by faide

ooops... or fixed in r3771 ? :)

comment:7 Changed 10 years ago by salfield

  • Status changed from closed to reopened
  • Version changed from 1.0.2 to trunk
  • Resolution fixed deleted
  • Severity changed from normal to blocker

The fix in changeset:3771 does not work for all possible (likely) nested variable structures. In particular in doesn't work where a dictionary is nested in a list which quite a common and desirable structure. Here is a solution which is more general.

         def encode_utf8(params):
            '''
            will recursively encode to utf-8 all values in a dictionnary
            what about when a dictionary is nested in a list?
            '''
            def encode_arg(value):
                if isinstance(value, dict):
                    for k,v in value.items():
                        value[k] = encode_arg(v)
                    return value
                elif isinstance(value, list):
                    return [encode_arg(vv) for vv in value]
                else:
                    return value.encode('utf-8')
            
            res = dict()
            for k, v in params.items():
                res[k] = encode_arg(v)
            return res

comment:8 Changed 10 years ago by chrisz

  • Status changed from reopened to closed
  • Resolution set to fixed
  • Severity changed from blocker to normal
  • Milestone changed from 1.1 to 1.0.4

@salfield: This has been addressed in #1622 already.

Note: See TracTickets for help on using tickets.