Ticket #1927 (closed enhancement: fixed)

Opened 2 years ago

Last modified 2 years ago

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

Reported by: Chris Arndt Assigned to: 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 (6.2 kB) - added by Chris Arndt on 08/06/08 20:00:31.
visit-from-params2.diff (8.0 kB) - added by Chris Arndt on 08/20/08 12:28:40.
New improved patch with changes suggested by comment no. 5

Change History

08/06/08 11:07:21 changed by Chris Arndt

  • type changed from defect to enhancement.

I'll propose a patch asap.

08/06/08 20:00:31 changed by Chris Arndt

  • attachment visit-from-params.diff added.

08/06/08 20:04:44 changed 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.

08/08/08 09:30:43 changed 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...

08/08/08 11:21:53 changed by droggisch

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

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

08/11/08 16:27:20 changed 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

08/20/08 12:24:12 changed by Chris Arndt

  • status changed from new to assigned.

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

08/20/08 12:28:40 changed by Chris Arndt

  • attachment visit-from-params2.diff added.

New improved patch with changes suggested by comment no. 5

08/20/08 16:46:30 changed by Chris Arndt

  • status changed from assigned to closed.
  • resolution set to fixed.

Backported to 1.1 branch in r5180

08/20/08 16:49:07 changed by Chris Arndt

Sorry, I meant the 1.0 branch.