#8808: Form Wizard keeps state on subsequent requests and provides no way to cut
short inside parse_params
---------------------------------------------------------------------+------
Reporter: Michael P. Jung |
Owner: nobody
Status: new |
Milestone: 1.0
Component: django.contrib.formtools |
Version: SVN
Keywords: FormWizard, state, parse_params, shortcut, HttpResponse |
Stage: Unreviewed
Has_patch: 1 |
---------------------------------------------------------------------+------
Since this issue is subtle I'll describe my concrete usecase first and
than describe the issue and possible solutions to this.
'''Usecase:'''
I have a form that contains a ChoiceField which depends on the currently
logged in user and some request parameters. Since I need a special
behaviour I subclassed the FormWizard and did overwrite the methods
{{{parse_params}}}, {{{done}}}, {{{process_step}}}, {{{get_form}}} and
{{{get_template}}}. Inside {{{parse_params}}} I set some attributes of
self. For some reason I assumed that returning anything but None would
cause the Wizard to abort. However the return value is ignored and the
wizard code continues by calling {{{get_form}}} and {{{process_step}}}.
Since {{{get_form}}} is the only way of putting the stuff which was set
earlier in {{{parse_params}}} I have a hook that calls some helper methods
on the form to set up the options of the ChoiceField depending on the
stuff set by {{{parse_params}}} earlier. Since the only way of injecting
custom data there is {{{self}}} one has no chance of telling wether the
value comes from the current request or something earlier which might just
have failed. Thus no AttributeError is raised as the attribute is set. The
data just does not come from the current request.
Even worse: In my case - since I was assuming that returning from
{{{parse_params}}} with an {{{HttpResponse}}} would stop the wizard and
return the HttpResponse immediately - caused invalid requests to be
rendered with the state from a previous requests. A horrid leak of
sensible data, as the wizard is part of a checkout process and the
{{{ChoiceField}}} is used to pick an address. I'm just glad our QA noticed
that glitch before putting this stuff live. It would not be hard to
exploit and I don't like the idea of leaking sensible data.
'''Solutions:'''
1. By overwriting the {{{__call__}}} method one can inject any attribute
into self and return any HttpResponse object if something is wrong. I
consider this rather ugly as the FormWizard explicitly states
{{{parse_params}}} for getting stuff from the request object, but provides
no way of breaking out of the wizard and returning a HttpResponse. I would
like to see a way to return a HttpResponse object from within
{{{parse_params}}} for shortcutting requests to a Wizard. (see attached
patch)
2. The Wizard could be made immutable, but provide a custom 'state'
attribute for transfering data between {{{parse_params}}},
{{{process_step}}}, etc. Anyhow this still would not enable the user of
the FormWizard to shortcut a request with a HttpRequest. I would consider
it a good idea not to be able to reuse a state from a previous request by
accident. Another idea would be to discourage the users to store such
state information as attribute, but rather enhance the form wizard to
implement {{{__getitem___}}}, {{{__setitem__}}}, etc. so that it can be
used to transfer state between the hook methods. Thus instead of doing
"{{{self.my_state = args[0]}}}" one would do "{{{self['my_state'] =
args[0]}}}".
3. In my case I solved this issue by wrapping the wizard inside a custom
request method which creates a new instance of the Form wizard on every
request. Maybe it would be a good idea to make it possible to have a class
inside the url config rather than an instance. e.g. the url handler code
could just check if the contained code is a class/type which needs to be
instanciated first. This would make it possible to get a fresh instance of
the object on every request.
'''Bottom line:'''
The FormWizard documentation needs to state that the Wizard object will be
reused. Thus one must be aware that subsequent requests might get the same
wizard instance. Thus any logic depending on a state set to {{{self}}}
should be considered unsafe. I can imagine quite a lot of people using the
FormWizard wrong without even knowing it.
I posted this as a 1.0 issue as I think this one should be figured out or
at least documented properly. It gave me a hard time and caused a critical
software bug that was hard to spot. It surely was my mistake to assume
that {{{parse_params}}} would be able to cut short the request, but the
way the FormWizard uses {{{self}}} to transfer state from one method to
another rounded it up to some rather unpleasing issue.
I attached a simple patch which enhances {{{parse_params}}} so that it is
capable of cutting short a request. The solutions (2.) is a bit more
complex and needs some tweaking of the source and changing of the
documentation so I did not provide a patch. Solution (3.) is just a quick
fix that I don't consider very pretty, but use it for now.
--
Ticket URL: <http://code.djangoproject.com/ticket/8808>
Django Code <http://code.djangoproject.com/>
The web framework for perfectionists with deadlines
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en
-~----------~----~----~----~------~----~------~--~---
