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 #1598 (closed defect: fixed)

Opened 9 years ago

Last modified 9 years ago

Passing non-ASCII characters as kwargs or nested params to method with @identity.require() produces decoding error when not logged in

Reported by: streawkceur Owned by: anonymous
Priority: normal Milestone: 1.0.4
Component: Identity Version: 1.0.4b3
Severity: normal Keywords: identity utf utf-8 nested parameters
Cc:

Description

Suppose I have this (minimal) controller:

class Root(controllers.RootController):
    @expose()
    def test(self, *args, **kwargs):
        return "bla"

    @expose()
    @identity.require(identity.not_anonymous())
    def test2(self, *args, **kwargs):
        return "bla2"

When I call /test?a=ä, all works as expected. But when I call the second handler /test2?a=ä, I get an internal server error. Note: This problem only seems to occur when the user actually is anonymous.

500 Internal error
[..]
  File "[..]python25\lib\site-packages\cherrypy-2.2.1-py2.5.egg\cherrypy\filters\decodingfilter.py", line 50, in decode
    decodedParams[key] = value.decode(enc)
  File "[..]python25\lib\encodings\utf_8.py", line 16, in decode
    return codecs.utf_8_decode(input, errors, True)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xe4' in position 0: ordinal not in range(128)

A possibly related problem occurs when calling the handler with "nested" query parameters: /test2?a.b=b

500 Internal error

The server encountered an unexpected condition which prevented it from
fulfilling the request.

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

The full test project (actually, only the controllers.py has been modified after the quickstart) is attached. It has been tested (only) with TG 1.0.4b2.

Attachments

tg-id-utf-bug.zip Download (95.4 KB) - added by streawkceur 9 years ago.
monkey-patch-decodingfilter.patch Download (588 bytes) - added by Chris Arndt 9 years ago.
Fix monkey-patching DecodeFilter? when no 'before_main' filters are present
monkey-decodingfilter.patch Download (880 bytes) - added by Chris Arndt 9 years ago.
Fix typo in MonkeyDecodingFilter?

Change History

Changed 9 years ago by streawkceur

comment:1 Changed 9 years ago by streawkceur

Adi has a patch that fixes the problem for my test case:

IN turbogears\visit\api.py
  IN class VisitFilter(BaseFilter):
    IN def before_main(self):

adding at the end of before_main (line 165 for TG 1.0.0.2):

for k,v in cherrypy.request.params.iteritems():
    cherrypy.request.params[k]=v.encode('utf-8')

See  http://groups.google.com/group/turbogears/browse_thread/thread/5bf6fdd53c87b07d

comment:2 Changed 9 years ago by streawkceur

Doesn't work with nested request params (i.e. foo.bar.baz) as they get converted into nested dicts, so the value v may be a dict that doesn't have a method named encode: AttributeError: 'dict' object has no attribute 'encode'.

To fix this fix, you have to do do a recursive conversion:

    def encode_utf8(params):
        for k, v in params.items():
            if type(v) is dict:
                encode_utf8(v)
            else:
                params[k] = v.encode('utf-8') 
    
    encode_utf8(cherrypy.request.params)

Funnily, you'll then get an error from CP:

  File "c:\programme\python25\lib\site-packages\cherrypy-2.2.1-py2.5.egg\cherrypy\filters\decodingfilter.py", line 50, in decode
    decodedParams[key] = value.decode(enc)
AttributeError: 'dict' object has no attribute 'decode'

But I've got a feeling that the real problems are located in another place, deeper in the system. But unfortunately I don't know much about TG's/CP's internals. Maybe this is ultimately a bug in CP. But I really don't know.

comment:3 Changed 9 years ago by faide

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

Fixed in r3762. Dicts are not managed because managed or not they make CP fail.

comment:4 Changed 9 years ago by streawkceur

  • Status changed from closed to reopened
  • Version changed from 1.0.4b1 to 1.0.4b2
  • Resolution fixed deleted

Actually, I still get an internal server error for my attached test project when invoking the URL  http://localhost:8080/test2?a.b=a

I'm using turbogears-1.0.4b2dev_r3766

500 Internal error

The server encountered an unexpected condition which prevented it from fulfilling the request.

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

comment:5 Changed 9 years ago by faide

Hi

This is normal CP does not seem to support dict args in its decoding filters... we cannot change this from here. Do you use such args as a.b=<someaccentedarg> ?

If yes then we need to investigate deeper down into CherryPy2's decodingfilters...

Florent.

comment:6 Changed 9 years ago by streawkceur

Yes, I do use these args as ToscaWidgets generates them.

ToscaWidgets generates form fields with dots in the names for nested widgets/form fields.

Simple example: Wrap some input fields (a,b,c) in a fieldset (fs). The input fields will have the names fs.a, fs.b, fs.c.

So with this bug you cannot use nested form elements in ToscaWidgets.

comment:7 Changed 9 years ago by faide

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

Fixed for real in #3771.

comment:8 Changed 9 years ago by roger.demetrescu

Correction: it is fixed in [3771]

comment:9 Changed 9 years ago by streawkceur

  • Status changed from closed to reopened
  • Keywords nested parameters added
  • Version changed from 1.0.4b2 to 1.0.4b3
  • Resolution fixed deleted

Still crashes in 3830. Example controller:

from turbogears import controllers, expose, flash
from turbogears import identity, redirect

class Root(controllers.RootController):
    @expose()
    def index(self):
        return '<html><body><a href="/test_wo_ident?a.b=c">Won\'t crash</a>, <a href="/test_with_ident?a.b=c">Will crash</a></body></html>'
    
    @expose()
    def test_wo_ident(self, **kwargs):
        return "works"
    
    @expose()
    @identity.require(identity.not_anonymous())
    def test_with_ident(self, **kwargs):
        return "crashes"
...
  File "/usr/lib/python2.4/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'

comment:10 Changed 9 years ago by chrisz

I can reproduce this. The unit tests have a test function (identity.tests.test_identity.test_decode_filter) that does the same thing as you were doing. One difference was that the unit test tested only the case with a successful login, while you use a wrong login. I have now handled the case of a wrong login in r3831 as well. The strange thing is that this still did not fix your bug, though the unit test does the same thing and it works. Maybe a difference between the testing environment and the standard environment? Will look into that later. Maybe somebody else has an idea.

comment:11 Changed 9 years ago by chrisz

Looks like the infamous monkey_before_main function is not called when you don't start from the test environment.

comment:12 Changed 9 years ago by Chris Arndt

  • Summary changed from Identity: Passing non-ASCII characters as kwargs to a handler with @identity.require(identity.not_anonymous()) produces internal server error to Passing non-ASCII characters as kwargs or nested params to method with @identity.require() produces decoding error when not logged in

comment:13 Changed 9 years ago by streawkceur

This one (maybe obviously) also crashes when called with nested params:

@expose()
def test(self, **kwargs):
    from turbogears.identity import IdentityFailure
    raise IdentityFailure, "boom"

So it's probably not the decorator but a raised identity exception that leads to the internal server error.

comment:14 Changed 9 years ago by faide

Hey Streawkceur,

This one we hope it is the good one :) please try this revision r3835. I tested with a test project the 3 exposed methods you proposed and all pass the test (manual test).

I did not have time to look at the automated tests yet to see why them fooled us.

Cheers, Florent.

PS: I keep this open because I want to make sure we have tests that validate something so we can guarantee we won't have regressions in the future.

comment:15 Changed 9 years ago by Chris Arndt

Hey Florent, great work! One issue though: your code for monky-patching DecodeFilter breaks when there is no 'before_main' key in cherrypy.filters._filterhooks. This happens when cherrypy.filters.init() isn't called. It should be called by cherrypy._cpengine.Engine.setup() but somehow this is not called (or called later?) when using turbogears.testutil.create_request(). The upshot is that your code works when running the CherryPy? server normally but breaks when you run the test suite in <your_package>/tests/test_controllers.py.

A simple patch is to use _filterhooks.get('before_main', []') for the for loop in turbogears.visit.api.start_extension (patch attached).

Changed 9 years ago by Chris Arndt

Fix monkey-patching DecodeFilter? when no 'before_main' filters are present

comment:16 Changed 9 years ago by faide

Thanks Chris,

This may be the reason why the tests did work differently than the server...

Could you apply this yourself to the 1.0 branch ? I won't have access to SVN 'till 22h30 or so and I suspect I won't have much time for coding tonight.

Florent.

Changed 9 years ago by Chris Arndt

Fix typo in MonkeyDecodingFilter?

comment:17 Changed 9 years ago by Chris Arndt

New patch that fixes a typo in MonkeyDecodingFilter and incorporates the previous patch.

I'll submit this to SVN, but we still need better tests. For example, the typo wasn't caught by the existing tests, I only stunbled accross this while testing #242.

comment:18 Changed 9 years ago by Chris Arndt

Applied in r2826. The commit msg is somewhat garbled, because I didn't escape the backticks in the shell properly ;-(

comment:19 Changed 9 years ago by Felix.Schwarz

I think you meant r3836 actually.

comment:20 Changed 9 years ago by Chris Arndt

Oops, of course. Off by one error ;-)

comment:21 Changed 9 years ago by faide

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

Fixed in 1.0.4. Fixed also the tests to really test if the params are unicode. r3974 & r3979

Note: See TracTickets for help on using tickets.