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 #1927 (closed enhancement: fixed)

Opened 9 years ago

Last modified 9 years ago

Visit framework should support getting the session ID from the request params instead a cookie

Reported by: Chris Arndt Owned by: Chris Arndt
Priority: normal Milestone: 1.5
Component: TurboGears Version: 1.0.4.4
Severity: normal Keywords:
Cc:

Description

Some buggy clients (e.g. some versions of Flash-based file uploaders) do not correctly submit the session cookie, so visit tracking and identity handling will fail for these request, since the each request will get a new session ID.

The visit framework currently only looks for the session ID in the request cookies. It should support getting the session ID from the request params as an alternative, so you can work around the buggy clients by transferring the session ID with the request params.

I'm proposing to change turbogears.visit.api.VisitFilter.before_main to retrieve the session key from the request according to a new configuration settings that specifies the methods to use and the order in which to try them. For example (in analogy to the identity.source setting):

visit.source = 'cookie,form'

For background information, see this mailing list thread:

 http://groups.google.com/group/turbogears/browse_frm/thread/f5f1e7331685f95d

Attachments

visit-from-params.diff Download (6.2 KB) - added by Chris Arndt 9 years ago.
visit-from-params2.diff Download (8.0 KB) - added by Chris Arndt 9 years ago.
New improved patch with changes suggested by comment no. 5

Change History

comment:1 Changed 9 years ago by Chris Arndt

  • Type changed from defect to enhancement

I'll propose a patch asap.

Changed 9 years ago by Chris Arndt

comment:2 Changed 9 years ago by Chris Arndt

Attched a patch that implements the change to the VisitFilter as described above (except I named the additional visit source in the configuration 'param'). Patch includes test and update to the quickstart configuration file template.

comment:3 Changed 9 years ago by faide

At first look it seems good. If no one handles this until I come back from holiday I'll take care of it next week...

comment:4 Changed 9 years ago by droggisch

I applied the patch and ran the test - everything worked flawlessly.

The review was also ok - so +1 from me for committing.

comment:5 Changed 9 years ago by chrisz

Patch seems good to me. Some minor issues:

  • Instead of param, I would use form, in analogy to identity.form, and instead of visit.param.visit_key, I would use visit.form.name, in analogy to visit.cookie.name.
  • If you want to backport this to 1.0, you must import Set as set for Python 2.3.
  • In the loop for source in self.source, the second break statement should come in an else block so that if you set source='param,cookie' and the param is not found, the cookie is checked (maybe this should be checked in the test as well).

The loop can also be simplified like that:

visit_key = None 
for source in self.source:
    if source == 'cookie': 
        visit_key = cherrypy.request.simple_cookie.get(self.cookie_name)
        if visit_key:
            visit_key = visit_key.value
    elif source == 'param':
        visit_key = cherrypy.request.params.pop(self.visit_key_param, None)
    if visit_key:
        visit = _manager.visit_for_key(visit_key)
        break

comment:6 Changed 9 years ago by Chris Arndt

  • Status changed from new to assigned

Applied in r5175 with the changes suggested in comment no. 5.

Changed 9 years ago by Chris Arndt

New improved patch with changes suggested by comment no. 5

comment:7 Changed 9 years ago by Chris Arndt

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

Backported to 1.1 branch in r5180

comment:8 Changed 9 years ago by Chris Arndt

Sorry, I meant the 1.0 branch.

Note: See TracTickets for help on using tickets.